lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 30 Dec 2022 17:29:57 +0700 From: Chanh Nguyen <chanh@...eremail.onmicrosoft.com> To: Andrzej Pietrasiewicz <andrzej.p@...labora.com>, Chanh Nguyen <chanh@...amperecomputing.com>, OpenBMC Maillist <openbmc@...ts.ozlabs.org>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Frank Li <frank.li@....com>, Christophe JAILLET <christophe.jaillet@...adoo.fr>, Dan Vacura <w36195@...orola.com>, Jakob Koschel <jakobkoschel@...il.com>, Alan Stern <stern@...land.harvard.edu>, Vijayavardhan Vennapusa <vvreddy@...eaurora.org>, Rondreis <linhaoguo86@...il.com>, linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org, Open Source Submission <patches@...erecomputing.com> Subject: Re: [PATCH v2] USB: gadget: Add ID numbers to configfs-gadget driver names On 22/12/2022 22:20, Andrzej Pietrasiewicz wrote: > Hi, > > W dniu 21.12.2022 o 10:13, Chanh Nguyen pisze: >> It is unable to use configfs to attach more than one gadget. When >> attaching the second gadget, it always fails and the kernel message >> prints out: >> >> Error: Driver 'configfs-gadget' is already registered, aborting... >> UDC core: g1: driver registration failed: -16 >> > > I assume you are interested in a scenario where there is more than one > UDC available which means you can have more than one active gadget? > Yes, I'd like to have more active gadgets. For example, mass_storage and ecm usb, ... >> This commit fixes the problem by a ".N" suffix added to each >> configfs_gadget's driver name (where N is a unique ID number), >> thus making the names distinct. >> >> Fixes: fc274c1e9973 ("USB: gadget: Add a new bus for gadgets") >> Signed-off-by: Chanh Nguyen <chanh@...amperecomputing.com> >> >> --- >> Changes in v2: >> - Replace scnprintf() by kasprintf() to simplify the code [CJ] >> - Move the clean up code from gadgets_drop() to >> gadget_info_attr_release() [Frank Li] >> - Correct the resource free up in gadges_make() [Alan Stern] >> - Remove the unnecessary variable in gadgets_make() [Chanh] >> - Fixes minor grammar issue in commit message [Chanh] >> --- >> drivers/usb/gadget/configfs.c | 25 ++++++++++++++++++++++++- >> 1 file changed, 24 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/usb/gadget/configfs.c >> b/drivers/usb/gadget/configfs.c >> index 96121d1c8df4..7faf68bfa716 100644 >> --- a/drivers/usb/gadget/configfs.c >> +++ b/drivers/usb/gadget/configfs.c >> @@ -3,6 +3,7 @@ >> #include <linux/module.h> >> #include <linux/slab.h> >> #include <linux/device.h> >> +#include <linux/idr.h> >> #include <linux/kstrtox.h> >> #include <linux/nls.h> >> #include <linux/usb/composite.h> >> @@ -11,6 +12,8 @@ >> #include "u_f.h" >> #include "u_os_desc.h" >> +static DEFINE_IDA(driver_id_numbers); >> + >> int check_user_usb_string(const char *name, >> struct usb_gadget_strings *stringtab_dev) >> { >> @@ -52,6 +55,9 @@ struct gadget_info { >> char qw_sign[OS_STRING_QW_SIGN_LEN]; >> spinlock_t spinlock; >> bool unbind; >> + >> + /* Make driver names unique */ >> + int driver_id_number; >> }; >> static inline struct gadget_info *to_gadget_info(struct config_item >> *item) >> @@ -393,6 +399,8 @@ static void gadget_info_attr_release(struct >> config_item *item) >> WARN_ON(!list_empty(&gi->string_list)); >> WARN_ON(!list_empty(&gi->available_func)); >> kfree(gi->composite.gadget_driver.function); >> + kfree(gi->composite.gadget_driver.driver.name); >> + ida_free(&driver_id_numbers, gi->driver_id_number); >> kfree(gi); >> } >> @@ -1623,13 +1631,28 @@ static struct config_group *gadgets_make( >> gi->composite.gadget_driver = configfs_driver_template; >> + gi->driver_id_number = ida_alloc(&driver_id_numbers, GFP_KERNEL); >> + if (gi->driver_id_number < 0) >> + goto err; >> + >> + gi->composite.gadget_driver.driver.name = kasprintf(GFP_KERNEL, >> + "configfs-gadget.%d", >> + gi->driver_id_number); > > I'm wondering if it maybe makes more sense to use the gadget name as a > suffix > instead? > > gi->composite.gadget_driver.driver.name = > kasprintf(GFP_KERNEL, "configfs-gadget.%s" name); > > So that when you > > mkdir g1 > > you will ultimately see /sys/bus/gadget/drivers/configfs-gadget.g1 > > instead of /sys/bus/gadget/drivers/configfs-gadget.0 > > Gadget names are guaranteed to be unique because they are created > as sibling subdirectories in configfs. Your patch would then be greatly > simplified (no need for ida). > > Regards, > > Andrzej > Thanks, Andrzej! I'll update that in patch v3 >> + if (!gi->composite.gadget_driver.driver.name) >> + goto out_free_driver_id_number; >> + >> gi->composite.gadget_driver.function = kstrdup(name, GFP_KERNEL); >> gi->composite.name = gi->composite.gadget_driver.function; >> if (!gi->composite.gadget_driver.function) >> - goto err; >> + goto out_free_driver_name; >> return &gi->group; >> + >> +out_free_driver_name: >> + kfree(gi->composite.gadget_driver.driver.name); >> +out_free_driver_id_number: >> + ida_free(&driver_id_numbers, gi->driver_id_number); >> err: >> kfree(gi); >> return ERR_PTR(-ENOMEM); >
Powered by blists - more mailing lists