Skip to content

Commit ff74745

Browse files
agabbasovFelipe Balbi
authored andcommitted
usb: gadget: configfs: Fix memory leak of interface directory data
Kmemleak checking configuration reports a memory leak in usb_os_desc_prepare_interf_dir function when rndis function instance is freed and then allocated again. For example, this happens with FunctionFS driver with RNDIS function enabled when "ffs-test" test application is run several times in a row. The data for intermediate "os_desc" group for interface directories is allocated as a single VLA chunk and (after a change of default groups handling) is not ever freed and actually not stored anywhere besides inside a list of default groups of a parent group. The fix is to make usb_os_desc_prepare_interf_dir function return a pointer to allocated data (as a pointer to the first VLA item) instead of (an unused) integer and to make the caller component (currently the only one is RNDIS function) responsible for storing the pointer and freeing the memory when appropriate. Fixes: 1ae1602 ("configfs: switch ->default groups to a linked list") Cc: [email protected] Signed-off-by: Andrew Gabbasov <[email protected]> Signed-off-by: Felipe Balbi <[email protected]>
1 parent aec17e1 commit ff74745

File tree

4 files changed

+25
-14
lines changed

4 files changed

+25
-14
lines changed

drivers/usb/gadget/configfs.c

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1143,11 +1143,12 @@ static struct configfs_attribute *interf_grp_attrs[] = {
11431143
NULL
11441144
};
11451145

1146-
int usb_os_desc_prepare_interf_dir(struct config_group *parent,
1147-
int n_interf,
1148-
struct usb_os_desc **desc,
1149-
char **names,
1150-
struct module *owner)
1146+
struct config_group *usb_os_desc_prepare_interf_dir(
1147+
struct config_group *parent,
1148+
int n_interf,
1149+
struct usb_os_desc **desc,
1150+
char **names,
1151+
struct module *owner)
11511152
{
11521153
struct config_group *os_desc_group;
11531154
struct config_item_type *os_desc_type, *interface_type;
@@ -1159,7 +1160,7 @@ int usb_os_desc_prepare_interf_dir(struct config_group *parent,
11591160

11601161
char *vlabuf = kzalloc(vla_group_size(data_chunk), GFP_KERNEL);
11611162
if (!vlabuf)
1162-
return -ENOMEM;
1163+
return ERR_PTR(-ENOMEM);
11631164

11641165
os_desc_group = vla_ptr(vlabuf, data_chunk, os_desc_group);
11651166
os_desc_type = vla_ptr(vlabuf, data_chunk, os_desc_type);
@@ -1184,7 +1185,7 @@ int usb_os_desc_prepare_interf_dir(struct config_group *parent,
11841185
configfs_add_default_group(&d->group, os_desc_group);
11851186
}
11861187

1187-
return 0;
1188+
return os_desc_group;
11881189
}
11891190
EXPORT_SYMBOL(usb_os_desc_prepare_interf_dir);
11901191

drivers/usb/gadget/configfs.h

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,12 @@
55

66
void unregister_gadget_item(struct config_item *item);
77

8-
int usb_os_desc_prepare_interf_dir(struct config_group *parent,
9-
int n_interf,
10-
struct usb_os_desc **desc,
11-
char **names,
12-
struct module *owner);
8+
struct config_group *usb_os_desc_prepare_interf_dir(
9+
struct config_group *parent,
10+
int n_interf,
11+
struct usb_os_desc **desc,
12+
char **names,
13+
struct module *owner);
1314

1415
static inline struct usb_os_desc *to_usb_os_desc(struct config_item *item)
1516
{

drivers/usb/gadget/function/f_rndis.c

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -908,6 +908,7 @@ static void rndis_free_inst(struct usb_function_instance *f)
908908
free_netdev(opts->net);
909909
}
910910

911+
kfree(opts->rndis_interf_group); /* single VLA chunk */
911912
kfree(opts);
912913
}
913914

@@ -916,6 +917,7 @@ static struct usb_function_instance *rndis_alloc_inst(void)
916917
struct f_rndis_opts *opts;
917918
struct usb_os_desc *descs[1];
918919
char *names[1];
920+
struct config_group *rndis_interf_group;
919921

920922
opts = kzalloc(sizeof(*opts), GFP_KERNEL);
921923
if (!opts)
@@ -940,8 +942,14 @@ static struct usb_function_instance *rndis_alloc_inst(void)
940942
names[0] = "rndis";
941943
config_group_init_type_name(&opts->func_inst.group, "",
942944
&rndis_func_type);
943-
usb_os_desc_prepare_interf_dir(&opts->func_inst.group, 1, descs,
944-
names, THIS_MODULE);
945+
rndis_interf_group =
946+
usb_os_desc_prepare_interf_dir(&opts->func_inst.group, 1, descs,
947+
names, THIS_MODULE);
948+
if (IS_ERR(rndis_interf_group)) {
949+
rndis_free_inst(&opts->func_inst);
950+
return ERR_CAST(rndis_interf_group);
951+
}
952+
opts->rndis_interf_group = rndis_interf_group;
945953

946954
return &opts->func_inst;
947955
}

drivers/usb/gadget/function/u_rndis.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ struct f_rndis_opts {
2626
bool bound;
2727
bool borrowed_net;
2828

29+
struct config_group *rndis_interf_group;
2930
struct usb_os_desc rndis_os_desc;
3031
char rndis_ext_compat_id[16];
3132

0 commit comments

Comments
 (0)