From 3e79ef2602cfa8209aa7f924469c2092108f596d Mon Sep 17 00:00:00 2001 From: Jack Pham Date: Tue, 1 Dec 2020 10:24:26 -0800 Subject: [PATCH 1/3] usb: gadget: f_ncm: Revert to upstream Revert the following changes to restore to upstream version: commit 006d8adf555a ("usb: gadget: f_ncm: allocate/free net device upon driver bind/unbind") commit ad2d551ece87 ("usb: gadget: Add check gadget function bind or not") Reason for revert: Causes gether_set_ifname() support to kernel panic. Change-Id: I51064467cad63e47a4a9734f18f1a5b95fa8db86 --- drivers/usb/gadget/function/f_ncm.c | 56 ++++++++----------- .../usb/gadget/function/u_ether_configfs.h | 35 ------------ 2 files changed, 23 insertions(+), 68 deletions(-) diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c index 03851c9c770a..f2627831757e 100644 --- a/drivers/usb/gadget/function/f_ncm.c +++ b/drivers/usb/gadget/function/f_ncm.c @@ -1470,38 +1470,18 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f) */ if (!ncm_opts->bound) { mutex_lock(&ncm_opts->lock); - ncm_opts->net = gether_setup_default(); - if (IS_ERR(ncm_opts->net)) { - status = PTR_ERR(ncm_opts->net); - mutex_unlock(&ncm_opts->lock); - goto error; - } gether_set_gadget(ncm_opts->net, cdev->gadget); status = gether_register_netdev(ncm_opts->net); mutex_unlock(&ncm_opts->lock); - if (status) { - free_netdev(ncm_opts->net); - goto error; - } + if (status) + goto fail; ncm_opts->bound = true; } - - /* export host's Ethernet address in CDC format */ - status = gether_get_host_addr_cdc(ncm_opts->net, ncm->ethaddr, - sizeof(ncm->ethaddr)); - if (status < 12) { /* strlen("01234567890a") */ - ERROR(cdev, "%s: failed to get host eth addr, err %d\n", - __func__, status); - status = -EINVAL; - goto netdev_cleanup; - } - ncm->port.ioport = netdev_priv(ncm_opts->net); - us = usb_gstrings_attach(cdev, ncm_strings, ARRAY_SIZE(ncm_string_defs)); if (IS_ERR(us)) { status = PTR_ERR(us); - goto netdev_cleanup; + goto fail; } ncm_control_intf.iInterface = us[STRING_CTRL_IDX].id; ncm_data_nop_intf.iInterface = us[STRING_DATA_IDX].id; @@ -1609,10 +1589,7 @@ fail: kfree(ncm->notify_req->buf); usb_ep_free_request(ncm->notify, ncm->notify_req); } -netdev_cleanup: - gether_cleanup(netdev_priv(ncm_opts->net)); -error: ERROR(cdev, "%s: can't bind, err %d\n", f->name, status); return status; @@ -1660,7 +1637,8 @@ static void ncm_free_inst(struct usb_function_instance *f) opts = container_of(f, struct f_ncm_opts, func_inst); if (opts->bound) gether_cleanup(netdev_priv(opts->net)); - + else + free_netdev(opts->net); kfree(opts->ncm_interf_group); kfree(opts); } @@ -1679,6 +1657,12 @@ static struct usb_function_instance *ncm_alloc_inst(void) mutex_init(&opts->lock); opts->func_inst.free_func_inst = ncm_free_inst; + opts->net = gether_setup_default(); + if (IS_ERR(opts->net)) { + struct net_device *net = opts->net; + kfree(opts); + return ERR_CAST(net); + } INIT_LIST_HEAD(&opts->ncm_os_desc.ext_prop); descs[0] = &opts->ncm_os_desc; @@ -1713,13 +1697,9 @@ static void ncm_free(struct usb_function *f) static void ncm_unbind(struct usb_configuration *c, struct usb_function *f) { struct f_ncm *ncm = func_to_ncm(f); - struct f_ncm_opts *opts = container_of(f->fi, struct f_ncm_opts, - func_inst); DBG(c->cdev, "ncm unbind\n"); - opts->bound = false; - hrtimer_cancel(&ncm->task_timer); kfree(f->os_desc_table); @@ -1735,14 +1715,13 @@ static void ncm_unbind(struct usb_configuration *c, struct usb_function *f) kfree(ncm->notify_req->buf); usb_ep_free_request(ncm->notify, ncm->notify_req); - - gether_cleanup(netdev_priv(opts->net)); } static struct usb_function *ncm_alloc(struct usb_function_instance *fi) { struct f_ncm *ncm; struct f_ncm_opts *opts; + int status; /* allocate and initialize one new instance */ ncm = kzalloc(sizeof(*ncm), GFP_KERNEL); @@ -1752,9 +1731,20 @@ static struct usb_function *ncm_alloc(struct usb_function_instance *fi) opts = container_of(fi, struct f_ncm_opts, func_inst); mutex_lock(&opts->lock); opts->refcnt++; + + /* export host's Ethernet address in CDC format */ + status = gether_get_host_addr_cdc(opts->net, ncm->ethaddr, + sizeof(ncm->ethaddr)); + if (status < 12) { /* strlen("01234567890a") */ + kfree(ncm); + mutex_unlock(&opts->lock); + return ERR_PTR(-EINVAL); + } ncm_string_defs[STRING_MAC_IDX].s = ncm->ethaddr; + spin_lock_init(&ncm->lock); ncm_reset_values(ncm); + ncm->port.ioport = netdev_priv(opts->net); mutex_unlock(&opts->lock); ncm->port.is_fixed = true; ncm->port.supports_multi_frame = true; diff --git a/drivers/usb/gadget/function/u_ether_configfs.h b/drivers/usb/gadget/function/u_ether_configfs.h index 82c9c85bd098..5b1d4771d44f 100644 --- a/drivers/usb/gadget/function/u_ether_configfs.h +++ b/drivers/usb/gadget/function/u_ether_configfs.h @@ -32,11 +32,6 @@ struct f_##_f_##_opts *opts = to_f_##_f_##_opts(item); \ int result; \ \ - if (opts->bound == false) { \ - pr_err("Gadget function do not bind yet.\n"); \ - return -ENODEV; \ - } \ - \ mutex_lock(&opts->lock); \ result = gether_get_dev_addr(opts->net, page, PAGE_SIZE); \ mutex_unlock(&opts->lock); \ @@ -50,11 +45,6 @@ struct f_##_f_##_opts *opts = to_f_##_f_##_opts(item); \ int ret; \ \ - if (opts->bound == false) { \ - pr_err("Gadget function do not bind yet.\n"); \ - return -ENODEV; \ - } \ - \ mutex_lock(&opts->lock); \ if (opts->refcnt) { \ mutex_unlock(&opts->lock); \ @@ -77,11 +67,6 @@ struct f_##_f_##_opts *opts = to_f_##_f_##_opts(item); \ int result; \ \ - if (opts->bound == false) { \ - pr_err("Gadget function do not bind yet.\n"); \ - return -ENODEV; \ - } \ - \ mutex_lock(&opts->lock); \ result = gether_get_host_addr(opts->net, page, PAGE_SIZE); \ mutex_unlock(&opts->lock); \ @@ -95,11 +80,6 @@ struct f_##_f_##_opts *opts = to_f_##_f_##_opts(item); \ int ret; \ \ - if (opts->bound == false) { \ - pr_err("Gadget function do not bind yet.\n"); \ - return -ENODEV; \ - } \ - \ mutex_lock(&opts->lock); \ if (opts->refcnt) { \ mutex_unlock(&opts->lock); \ @@ -122,11 +102,6 @@ struct f_##_f_##_opts *opts = to_f_##_f_##_opts(item); \ unsigned qmult; \ \ - if (opts->bound == false) { \ - pr_err("Gadget function do not bind yet.\n"); \ - return -ENODEV; \ - } \ - \ mutex_lock(&opts->lock); \ qmult = gether_get_qmult(opts->net); \ mutex_unlock(&opts->lock); \ @@ -140,11 +115,6 @@ u8 val; \ int ret; \ \ - if (opts->bound == false) { \ - pr_err("Gadget function do not bind yet.\n"); \ - return -ENODEV; \ - } \ - \ mutex_lock(&opts->lock); \ if (opts->refcnt) { \ ret = -EBUSY; \ @@ -171,11 +141,6 @@ out: \ struct f_##_f_##_opts *opts = to_f_##_f_##_opts(item); \ int ret; \ \ - if (opts->bound == false) { \ - pr_err("Gadget function do not bind yet.\n"); \ - return -ENODEV; \ - } \ - \ mutex_lock(&opts->lock); \ ret = gether_get_ifname(opts->net, page, PAGE_SIZE); \ mutex_unlock(&opts->lock); \ From 305e9e4cf9ee200ed6db7bfd2adeec18145ff71d Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Thu, 14 Jan 2021 08:42:22 +0900 Subject: [PATCH 2/3] FROMGIT: usb: gadget: u_ether: support configuring interface names. This patch allows the administrator to configure the interface name of a function using u_ether (e.g., eem, ncm, rndis). Currently, all such interfaces, regardless of function type, are always called usb0, usb1, etc. This makes it very cumbersome to use more than one such type at a time, because userspace cannnot easily tell the interfaces apart and apply the right configuration to each one. Interface renaming in userspace based on driver doesn't help, because the interfaces all have the same driver. Without this patch, doing this require hacks/workarounds such as setting fixed MAC addresses on the functions, and then renaming by MAC address, or scraping configfs after each interface is created to find out what it is. Setting the interface name is done by writing to the same "ifname" configfs attribute that reports the interface name after the function is bound. The write must contain an interface pattern such as "usb%d" (which will cause the net core to pick the next available interface name starting with "usb"). This patch does not allow writing an exact interface name (as opposed to a pattern) because if the interface already exists at bind time, the bind will fail and the whole gadget will fail to activate. This could be allowed in a future patch. For compatibility with current userspace, when reading an ifname that has not currently been set, the result is still "(unnamed net_device)". Once a write to ifname happens, then reading ifname will return whatever was last written. Tested by configuring an rndis function and an ncm function on the same gadget, and writing "rndis%d" to ifname on the rndis function and "ncm%d" to ifname on the ncm function. When the gadget was bound, the rndis interface was rndis0 and the ncm interface was ncm0. Signed-off-by: Lorenzo Colitti (cherry picked from commit 63d152149b2d0860ccf8c4e6596b6175b2b7ace6 https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-next) Link: https://lore.kernel.org/r/20210113234222.3272933-1-lorenzo@google.com Signed-off-by: Greg Kroah-Hartman Signed-off-by: Lorenzo Colitti Change-Id: I04deb6cc1d8a5b8ee82404940de2a79c06fbafe7 Signed-off-by: Greg Kroah-Hartman --- Documentation/usb/gadget-testing.rst | 30 ++++++++--------- drivers/usb/gadget/function/u_ether.c | 33 ++++++++++++++++++- drivers/usb/gadget/function/u_ether.h | 12 +++++++ .../usb/gadget/function/u_ether_configfs.h | 15 ++++++++- 4 files changed, 73 insertions(+), 17 deletions(-) diff --git a/Documentation/usb/gadget-testing.rst b/Documentation/usb/gadget-testing.rst index 2eeb3e9299e4..2085e7b24eeb 100644 --- a/Documentation/usb/gadget-testing.rst +++ b/Documentation/usb/gadget-testing.rst @@ -91,9 +91,9 @@ The ECM function provides these attributes in its function directory: and after creating the functions/ecm. they contain default values: qmult is 5, dev_addr and host_addr are randomly selected. -Except for ifname they can be written to until the function is linked to a -configuration. The ifname is read-only and contains the name of the interface -which was assigned by the net core, e. g. usb0. +The ifname can be written to if the function is not bound. A write must be an +interface pattern such as "usb%d", which will cause the net core to choose the +next free usbX interface. By default, it is set to "usb%d". Testing the ECM function ------------------------ @@ -131,9 +131,9 @@ The ECM subset function provides these attributes in its function directory: and after creating the functions/ecm. they contain default values: qmult is 5, dev_addr and host_addr are randomly selected. -Except for ifname they can be written to until the function is linked to a -configuration. The ifname is read-only and contains the name of the interface -which was assigned by the net core, e. g. usb0. +The ifname can be written to if the function is not bound. A write must be an +interface pattern such as "usb%d", which will cause the net core to choose the +next free usbX interface. By default, it is set to "usb%d". Testing the ECM subset function ------------------------------- @@ -171,9 +171,9 @@ The EEM function provides these attributes in its function directory: and after creating the functions/eem. they contain default values: qmult is 5, dev_addr and host_addr are randomly selected. -Except for ifname they can be written to until the function is linked to a -configuration. The ifname is read-only and contains the name of the interface -which was assigned by the net core, e. g. usb0. +The ifname can be written to if the function is not bound. A write must be an +interface pattern such as "usb%d", which will cause the net core to choose the +next free usbX interface. By default, it is set to "usb%d". Testing the EEM function ------------------------ @@ -453,9 +453,9 @@ The NCM function provides these attributes in its function directory: and after creating the functions/ncm. they contain default values: qmult is 5, dev_addr and host_addr are randomly selected. -Except for ifname they can be written to until the function is linked to a -configuration. The ifname is read-only and contains the name of the interface -which was assigned by the net core, e. g. usb0. +The ifname can be written to if the function is not bound. A write must be an +interface pattern such as "usb%d", which will cause the net core to choose the +next free usbX interface. By default, it is set to "usb%d". Testing the NCM function ------------------------ @@ -591,9 +591,9 @@ The RNDIS function provides these attributes in its function directory: and after creating the functions/rndis. they contain default values: qmult is 5, dev_addr and host_addr are randomly selected. -Except for ifname they can be written to until the function is linked to a -configuration. The ifname is read-only and contains the name of the interface -which was assigned by the net core, e. g. usb0. +The ifname can be written to if the function is not bound. A write must be an +interface pattern such as "usb%d", which will cause the net core to choose the +next free usbX interface. By default, it is set to "usb%d". Testing the RNDIS function -------------------------- diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c index e11673599e55..3eedaf25c5a1 100644 --- a/drivers/usb/gadget/function/u_ether.c +++ b/drivers/usb/gadget/function/u_ether.c @@ -82,6 +82,7 @@ struct eth_dev { bool zlp; bool no_skb_reserve; + bool ifname_set; u8 host_mac[ETH_ALEN]; u8 dev_mac[ETH_ALEN]; }; @@ -979,15 +980,45 @@ EXPORT_SYMBOL_GPL(gether_get_qmult); int gether_get_ifname(struct net_device *net, char *name, int len) { + struct eth_dev *dev = netdev_priv(net); int ret; rtnl_lock(); - ret = scnprintf(name, len, "%s\n", netdev_name(net)); + ret = scnprintf(name, len, "%s\n", + dev->ifname_set ? net->name : netdev_name(net)); rtnl_unlock(); return ret; } EXPORT_SYMBOL_GPL(gether_get_ifname); +int gether_set_ifname(struct net_device *net, const char *name, int len) +{ + struct eth_dev *dev = netdev_priv(net); + char tmp[IFNAMSIZ]; + const char *p; + + if (name[len - 1] == '\n') + len--; + + if (len >= sizeof(tmp)) + return -E2BIG; + + strscpy(tmp, name, len + 1); + if (!dev_valid_name(tmp)) + return -EINVAL; + + /* Require exactly one %d, so binding will not fail with EEXIST. */ + p = strchr(name, '%'); + if (!p || p[1] != 'd' || strchr(p + 2, '%')) + return -EINVAL; + + strncpy(net->name, tmp, sizeof(net->name)); + dev->ifname_set = true; + + return 0; +} +EXPORT_SYMBOL_GPL(gether_set_ifname); + /** * gether_cleanup - remove Ethernet-over-USB device * Context: may sleep diff --git a/drivers/usb/gadget/function/u_ether.h b/drivers/usb/gadget/function/u_ether.h index 332307d54292..18eb89da6cb9 100644 --- a/drivers/usb/gadget/function/u_ether.h +++ b/drivers/usb/gadget/function/u_ether.h @@ -244,6 +244,18 @@ unsigned gether_get_qmult(struct net_device *net); */ int gether_get_ifname(struct net_device *net, char *name, int len); +/** + * gether_set_ifname - set an ethernet-over-usb link interface name + * @net: device representing this link + * @name: new interface name + * @len: length of @name + * + * This sets the interface name of this ethernet-over-usb link. + * A single terminating newline, if any, is ignored. + * Returns zero on success, else negative errno. + */ +int gether_set_ifname(struct net_device *net, const char *name, int len); + void gether_cleanup(struct eth_dev *dev); /* connect/disconnect is handled by individual functions */ diff --git a/drivers/usb/gadget/function/u_ether_configfs.h b/drivers/usb/gadget/function/u_ether_configfs.h index 5b1d4771d44f..f9f5adfef786 100644 --- a/drivers/usb/gadget/function/u_ether_configfs.h +++ b/drivers/usb/gadget/function/u_ether_configfs.h @@ -148,7 +148,20 @@ out: \ return ret; \ } \ \ - CONFIGFS_ATTR_RO(_f_##_opts_, ifname) + static ssize_t _f_##_opts_ifname_store(struct config_item *item, \ + const char *page, size_t len)\ + { \ + struct f_##_f_##_opts *opts = to_f_##_f_##_opts(item); \ + int ret = -EBUSY; \ + \ + mutex_lock(&opts->lock); \ + if (!opts->refcnt) \ + ret = gether_set_ifname(opts->net, page, len); \ + mutex_unlock(&opts->lock); \ + return ret ?: len; \ + } \ + \ + CONFIGFS_ATTR(_f_##_opts_, ifname) #define USB_ETHER_CONFIGFS_ITEM_ATTR_U8_RW(_f_, _n_) \ static ssize_t _f_##_opts_##_n_##_show(struct config_item *item,\ From 0db4cc475df658591d8435ad1c92208e812e755c Mon Sep 17 00:00:00 2001 From: Hardik Gajjar Date: Fri, 20 Oct 2023 17:33:24 +0200 Subject: [PATCH 3/3] usb: gadget: f_ncm: Always set current gadget in ncm_bind() [ Upstream commit a04224da1f3424b2c607b12a3bd1f0e302fb8231 ] Previously, gadget assignment to the net device occurred exclusively during the initial binding attempt. Nevertheless, the gadget pointer could change during bind/unbind cycles due to various conditions, including the unloading/loading of the UDC device driver or the detachment/reconnection of an OTG-capable USB hub device. This patch relocates the gether_set_gadget() function out from ncm_opts->bound condition check, ensuring that the correct gadget is assigned during each bind request. The provided logs demonstrate the consistency of ncm_opts throughout the power cycle, while the gadget may change. * OTG hub connected during boot up and assignment of gadget and ncm_opts pointer [ 2.366301] usb 2-1.5: New USB device found, idVendor=2996, idProduct=0105 [ 2.366304] usb 2-1.5: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 2.366306] usb 2-1.5: Product: H2H Bridge [ 2.366308] usb 2-1.5: Manufacturer: Aptiv [ 2.366309] usb 2-1.5: SerialNumber: 13FEB2021 [ 2.427989] usb 2-1.5: New USB device found, VID=2996, PID=0105 [ 2.428959] dabridge 2-1.5:1.0: dabridge 2-4 total endpoints=5, 0000000093a8d681 [ 2.429710] dabridge 2-1.5:1.0: P(0105) D(22.06.22) F(17.3.16) H(1.1) high-speed [ 2.429714] dabridge 2-1.5:1.0: Hub 2-2 P(0151) V(06.87) [ 2.429956] dabridge 2-1.5:1.0: All downstream ports in host mode [ 2.430093] gadget 000000003c414d59 ------> gadget pointer * NCM opts and associated gadget pointer during First ncm_bind [ 34.763929] NCM opts 00000000aa304ac9 [ 34.763930] NCM gadget 000000003c414d59 * OTG capable hub disconnecte or assume driver unload. [ 97.203114] usb 2-1: USB disconnect, device number 2 [ 97.203118] usb 2-1.1: USB disconnect, device number 3 [ 97.209217] usb 2-1.5: USB disconnect, device number 4 [ 97.230990] dabr_udc deleted * Reconnect the OTG hub or load driver assaign new gadget pointer. [ 111.534035] usb 2-1.1: New USB device found, idVendor=2996, idProduct=0120, bcdDevice= 6.87 [ 111.534038] usb 2-1.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 111.534040] usb 2-1.1: Product: Vendor [ 111.534041] usb 2-1.1: Manufacturer: Aptiv [ 111.534042] usb 2-1.1: SerialNumber: Superior [ 111.535175] usb 2-1.1: New USB device found, VID=2996, PID=0120 [ 111.610995] usb 2-1.5: new high-speed USB device number 8 using xhci-hcd [ 111.630052] usb 2-1.5: New USB device found, idVendor=2996, idProduct=0105, bcdDevice=21.02 [ 111.630055] usb 2-1.5: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 111.630057] usb 2-1.5: Product: H2H Bridge [ 111.630058] usb 2-1.5: Manufacturer: Aptiv [ 111.630059] usb 2-1.5: SerialNumber: 13FEB2021 [ 111.687464] usb 2-1.5: New USB device found, VID=2996, PID=0105 [ 111.690375] dabridge 2-1.5:1.0: dabridge 2-8 total endpoints=5, 000000000d87c961 [ 111.691172] dabridge 2-1.5:1.0: P(0105) D(22.06.22) F(17.3.16) H(1.1) high-speed [ 111.691176] dabridge 2-1.5:1.0: Hub 2-6 P(0151) V(06.87) [ 111.691646] dabridge 2-1.5:1.0: All downstream ports in host mode [ 111.692298] gadget 00000000dc72f7a9 --------> new gadget ptr on connect * NCM opts and associated gadget pointer during second ncm_bind [ 113.271786] NCM opts 00000000aa304ac9 -----> same opts ptr used during first bind [ 113.271788] NCM gadget 00000000dc72f7a9 ----> however new gaget ptr, that will not set in net_device due to ncm_opts->bound = true Change-Id: I803f892ccd2a0f9558d4f32d8a3104aba78353ff Signed-off-by: Hardik Gajjar Link: https://lore.kernel.org/r/20231020153324.82794-1-hgajjar@de.adit-jv.com Signed-off-by: Greg Kroah-Hartman Signed-off-by: Sasha Levin --- drivers/usb/gadget/function/f_ncm.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c index f2627831757e..ca50257b9538 100644 --- a/drivers/usb/gadget/function/f_ncm.c +++ b/drivers/usb/gadget/function/f_ncm.c @@ -1461,22 +1461,17 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f) f->os_desc_table[0].os_desc = &ncm_opts->ncm_os_desc; } - /* - * in drivers/usb/gadget/configfs.c:configfs_composite_bind() - * configurations are bound in sequence with list_for_each_entry, - * in each configuration its functions are bound in sequence - * with list_for_each_entry, so we assume no race condition - * with regard to ncm_opts->bound access - */ - if (!ncm_opts->bound) { - mutex_lock(&ncm_opts->lock); - gether_set_gadget(ncm_opts->net, cdev->gadget); + mutex_lock(&ncm_opts->lock); + gether_set_gadget(ncm_opts->net, cdev->gadget); + if (!ncm_opts->bound) status = gether_register_netdev(ncm_opts->net); - mutex_unlock(&ncm_opts->lock); - if (status) - goto fail; - ncm_opts->bound = true; - } + mutex_unlock(&ncm_opts->lock); + + if (status) + goto fail; + + ncm_opts->bound = true; + us = usb_gstrings_attach(cdev, ncm_strings, ARRAY_SIZE(ncm_string_defs)); if (IS_ERR(us)) {