usb: gadget: configfs: Fix memory leak of interface directory data

BugLink: http://bugs.launchpad.net/bugs/1724669
commit ff74745e6d3d97a865eda8c1f3fd29c13b79f0cc upstream.

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: 1ae1602de028 ("configfs: switch ->default groups to a linked list")
Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Seth Forshee <seth.forshee@canonical.com>

diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index a22a892de7b7e0ae3a1d816aa943bcac053880a4..aeb9f3c4052157e9a99d9c5dceef026970ea76f3 100644 (file)
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -1143,11 +1143,12 @@ static struct configfs_attribute *interf_grp_attrs[] = {
        NULL
 };
 
-int usb_os_desc_prepare_interf_dir(struct config_group *parent,
-                                  int n_interf,
-                                  struct usb_os_desc **desc,
-                                  char **names,
-                                  struct module *owner)
+struct config_group *usb_os_desc_prepare_interf_dir(
+               struct config_group *parent,
+               int n_interf,
+               struct usb_os_desc **desc,
+               char **names,
+               struct module *owner)
 {
        struct config_group *os_desc_group;
        struct config_item_type *os_desc_type, *interface_type;
@@ -1159,7 +1160,7 @@ int usb_os_desc_prepare_interf_dir(struct config_group *parent,
 
        char *vlabuf = kzalloc(vla_group_size(data_chunk), GFP_KERNEL);
        if (!vlabuf)
-               return -ENOMEM;
+               return ERR_PTR(-ENOMEM);
 
        os_desc_group = vla_ptr(vlabuf, data_chunk, os_desc_group);
        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,
                configfs_add_default_group(&d->group, os_desc_group);
        }
 
-       return 0;
+       return os_desc_group;
 }
 EXPORT_SYMBOL(usb_os_desc_prepare_interf_dir);
 

diff --git a/drivers/usb/gadget/configfs.h b/drivers/usb/gadget/configfs.h
index 36c468c4f5e90be54e7356d48dc2037476a62994..540d5e92ed2259149f736f4bd60e154ccff6bb37 100644 (file)
--- a/drivers/usb/gadget/configfs.h
+++ b/drivers/usb/gadget/configfs.h
@@ -5,11 +5,12 @@
 
 void unregister_gadget_item(struct config_item *item);
 
-int usb_os_desc_prepare_interf_dir(struct config_group *parent,
-                                  int n_interf,
-                                  struct usb_os_desc **desc,
-                                  char **names,
-                                  struct module *owner);
+struct config_group *usb_os_desc_prepare_interf_dir(
+               struct config_group *parent,
+               int n_interf,
+               struct usb_os_desc **desc,
+               char **names,
+               struct module *owner);
 
 static inline struct usb_os_desc *to_usb_os_desc(struct config_item *item)
 {

diff --git a/drivers/usb/gadget/function/f_rndis.c b/drivers/usb/gadget/function/f_rndis.c
index 16562e46112179b419a353a6508b6629c7d10d7e..ba00cdb809d6584a143605bc1211e8cb9ea407f1 100644 (file)
--- a/drivers/usb/gadget/function/f_rndis.c
+++ b/drivers/usb/gadget/function/f_rndis.c
@@ -892,6 +892,7 @@ static void rndis_free_inst(struct usb_function_instance *f)
                        free_netdev(opts->net);
        }
 
+       kfree(opts->rndis_interf_group);        /* single VLA chunk */
        kfree(opts);
 }
 
@@ -900,6 +901,7 @@ static struct usb_function_instance *rndis_alloc_inst(void)
        struct f_rndis_opts *opts;
        struct usb_os_desc *descs[1];
        char *names[1];
+       struct config_group *rndis_interf_group;
 
        opts = kzalloc(sizeof(*opts), GFP_KERNEL);
        if (!opts)
@@ -920,8 +922,14 @@ static struct usb_function_instance *rndis_alloc_inst(void)
        names[0] = "rndis";
        config_group_init_type_name(&opts->func_inst.group, "",
                                    &rndis_func_type);
-       usb_os_desc_prepare_interf_dir(&opts->func_inst.group, 1, descs,
-                                      names, THIS_MODULE);
+       rndis_interf_group =
+               usb_os_desc_prepare_interf_dir(&opts->func_inst.group, 1, descs,
+                                              names, THIS_MODULE);
+       if (IS_ERR(rndis_interf_group)) {
+               rndis_free_inst(&opts->func_inst);
+               return ERR_CAST(rndis_interf_group);
+       }
+       opts->rndis_interf_group = rndis_interf_group;
 
        return &opts->func_inst;
 }

diff --git a/drivers/usb/gadget/function/u_rndis.h b/drivers/usb/gadget/function/u_rndis.h
index 4eafd50505458cf10d2b58e48b8ba1dcd092ed4d..4e2ad04fe8d65b767aa2db70892e32671eb14f8f 100644 (file)
--- a/drivers/usb/gadget/function/u_rndis.h
+++ b/drivers/usb/gadget/function/u_rndis.h
@@ -26,6 +26,7 @@ struct f_rndis_opts {
        bool                            bound;
        bool                            borrowed_net;
 
+       struct config_group             *rndis_interf_group;
        struct usb_os_desc              rndis_os_desc;
        char                            rndis_ext_compat_id[16];