usb: gadget: f_ncm: align net_device lifecycle with bind/unbind

Currently, the net_device is allocated in ncm_alloc_inst() and freed in
ncm_free_inst(). This ties the network interface's lifetime to the
configuration instance rather than the USB connection (bind/unbind).

This decoupling causes issues when the USB gadget is disconnected where
the underlying gadget device is removed. The net_device can outlive its
parent, leading to dangling sysfs links and NULL pointer dereferences
when accessing the freed gadget device.

Problem 1: NULL pointer dereference on disconnect
 Unable to handle kernel NULL pointer dereference at virtual address
 0000000000000000
 Call trace:
   __pi_strlen+0x14/0x150
   rtnl_fill_ifinfo+0x6b4/0x708
   rtmsg_ifinfo_build_skb+0xd8/0x13c
   rtmsg_ifinfo+0x50/0xa0
   __dev_notify_flags+0x4c/0x1f0
   dev_change_flags+0x54/0x70
   do_setlink+0x390/0xebc
   rtnl_newlink+0x7d0/0xac8
   rtnetlink_rcv_msg+0x27c/0x410
   netlink_rcv_skb+0x134/0x150
   rtnetlink_rcv+0x18/0x28
   netlink_unicast+0x254/0x3f0
   netlink_sendmsg+0x2e0/0x3d4

Problem 2: Dangling sysfs symlinks
 console:/ # ls -l /sys/class/net/ncm0
 lrwxrwxrwx ... /sys/class/net/ncm0 ->
 /sys/devices/platform/.../gadget.0/net/ncm0
 console:/ # ls -l /sys/devices/platform/.../gadget.0/net/ncm0
 ls: .../gadget.0/net/ncm0: No such file or directory

Move the net_device allocation to ncm_bind() and deallocation to
ncm_unbind(). This ensures the network interface exists only when the
gadget function is actually bound to a configuration.

To support pre-bind configuration (e.g., setting interface name or MAC
address via configfs), cache user-provided options in f_ncm_opts
using the gether_opts structure. Apply these cached settings to the
net_device upon creation in ncm_bind().

Preserve the use-after-free fix from commit 6334b8e4553c ("usb: gadget:
f_ncm: Fix UAF ncm object at re-bind after usb ep transport error").
Check opts->net in ncm_set_alt() and ncm_disable() to ensure
gether_disconnect() runs only if a connection was established.

Fixes: 40d133d7f542 ("usb: gadget: f_ncm: convert to new function interface with backward compatibility")
Cc: stable@kernel.org
Signed-off-by: Kuen-Han Tsai <khtsai@google.com>
Link: https://patch.msgid.link/20251230-ncm-refactor-v1-3-793e347bc7a7@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c
index 0e38330271d5ac40f7f46b7e8d565c1f0d41e4b8..e23adc132f8865f6bbce6c88c8b5f3f06110faaa 100644 (file)
--- a/drivers/usb/gadget/function/f_ncm.c
+++ b/drivers/usb/gadget/function/f_ncm.c
@@ -83,6 +83,11 @@ static inline struct f_ncm *func_to_ncm(struct usb_function *f)
        return container_of(f, struct f_ncm, port.func);
 }
 
+static inline struct f_ncm_opts *func_to_ncm_opts(struct usb_function *f)
+{
+       return container_of(f->fi, struct f_ncm_opts, func_inst);
+}
+
 /*-------------------------------------------------------------------------*/
 
 /*
@@ -859,6 +864,7 @@ invalid:
 static int ncm_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 {
        struct f_ncm            *ncm = func_to_ncm(f);
+       struct f_ncm_opts       *opts = func_to_ncm_opts(f);
        struct usb_composite_dev *cdev = f->config->cdev;
 
        /* Control interface has only altsetting 0 */
@@ -881,12 +887,13 @@ static int ncm_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
                if (alt > 1)
                        goto fail;
 
-               if (ncm->netdev) {
-                       DBG(cdev, "reset ncm\n");
-                       ncm->netdev = NULL;
-                       gether_disconnect(&ncm->port);
-                       ncm_reset_values(ncm);
-               }
+               scoped_guard(mutex, &opts->lock)
+                       if (opts->net) {
+                               DBG(cdev, "reset ncm\n");
+                               opts->net = NULL;
+                               gether_disconnect(&ncm->port);
+                               ncm_reset_values(ncm);
+                       }
 
                /*
                 * CDC Network only sends data in non-default altsettings.
@@ -919,7 +926,8 @@ static int ncm_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
                        net = gether_connect(&ncm->port);
                        if (IS_ERR(net))
                                return PTR_ERR(net);
-                       ncm->netdev = net;
+                       scoped_guard(mutex, &opts->lock)
+                               opts->net = net;
                }
 
                spin_lock(&ncm->lock);
@@ -1366,14 +1374,16 @@ err:
 static void ncm_disable(struct usb_function *f)
 {
        struct f_ncm            *ncm = func_to_ncm(f);
+       struct f_ncm_opts       *opts = func_to_ncm_opts(f);
        struct usb_composite_dev *cdev = f->config->cdev;
 
        DBG(cdev, "ncm deactivated\n");
 
-       if (ncm->netdev) {
-               ncm->netdev = NULL;
-               gether_disconnect(&ncm->port);
-       }
+       scoped_guard(mutex, &opts->lock)
+               if (opts->net) {
+                       opts->net = NULL;
+                       gether_disconnect(&ncm->port);
+               }
 
        if (ncm->notify->enabled) {
                usb_ep_disable(ncm->notify);
@@ -1433,39 +1443,44 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f)
 {
        struct usb_composite_dev *cdev = c->cdev;
        struct f_ncm            *ncm = func_to_ncm(f);
+       struct f_ncm_opts       *ncm_opts = func_to_ncm_opts(f);
        struct usb_string       *us;
        int                     status = 0;
        struct usb_ep           *ep;
-       struct f_ncm_opts       *ncm_opts;
 
        struct usb_os_desc_table        *os_desc_table __free(kfree) = NULL;
+       struct net_device               *netdev __free(free_gether_netdev) = NULL;
        struct usb_request              *request __free(free_usb_request) = NULL;
 
        if (!can_support_ecm(cdev->gadget))
                return -EINVAL;
 
-       ncm_opts = container_of(f->fi, struct f_ncm_opts, func_inst);
-
        if (cdev->use_os_string) {
                os_desc_table = kzalloc(sizeof(*os_desc_table), GFP_KERNEL);
                if (!os_desc_table)
                        return -ENOMEM;
        }
 
-       mutex_lock(&ncm_opts->lock);
-       gether_set_gadget(ncm_opts->net, cdev->gadget);
-       if (!ncm_opts->bound) {
-               ncm_opts->net->mtu = (ncm_opts->max_segment_size - ETH_HLEN);
-               status = gether_register_netdev(ncm_opts->net);
+       netdev = gether_setup_default();
+       if (IS_ERR(netdev))
+               return -ENOMEM;
+
+       scoped_guard(mutex, &ncm_opts->lock) {
+               gether_apply_opts(netdev, &ncm_opts->net_opts);
+               netdev->mtu = ncm_opts->max_segment_size - ETH_HLEN;
        }
-       mutex_unlock(&ncm_opts->lock);
 
+       gether_set_gadget(netdev, cdev->gadget);
+       status = gether_register_netdev(netdev);
        if (status)
                return status;
 
-       ncm_opts->bound = true;
-
-       ncm_string_defs[1].s = ncm->ethaddr;
+       /* export host's Ethernet address in CDC format */
+       status = gether_get_host_addr_cdc(netdev, ncm->ethaddr,
+                                         sizeof(ncm->ethaddr));
+       if (status < 12)
+               return -EINVAL;
+       ncm_string_defs[STRING_MAC_IDX].s = ncm->ethaddr;
 
        us = usb_gstrings_attach(cdev, ncm_strings,
                                 ARRAY_SIZE(ncm_string_defs));
@@ -1563,6 +1578,8 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f)
                f->os_desc_n = 1;
        }
        ncm->notify_req = no_free_ptr(request);
+       ncm->netdev = no_free_ptr(netdev);
+       ncm->port.ioport = netdev_priv(ncm->netdev);
 
        DBG(cdev, "CDC Network: IN/%s OUT/%s NOTIFY/%s\n",
                        ncm->port.in_ep->name, ncm->port.out_ep->name,
@@ -1577,19 +1594,19 @@ static inline struct f_ncm_opts *to_f_ncm_opts(struct config_item *item)
 }
 
 /* f_ncm_item_ops */
-USB_ETHERNET_CONFIGFS_ITEM(ncm);
+USB_ETHER_OPTS_ITEM(ncm);
 
 /* f_ncm_opts_dev_addr */
-USB_ETHERNET_CONFIGFS_ITEM_ATTR_DEV_ADDR(ncm);
+USB_ETHER_OPTS_ATTR_DEV_ADDR(ncm);
 
 /* f_ncm_opts_host_addr */
-USB_ETHERNET_CONFIGFS_ITEM_ATTR_HOST_ADDR(ncm);
+USB_ETHER_OPTS_ATTR_HOST_ADDR(ncm);
 
 /* f_ncm_opts_qmult */
-USB_ETHERNET_CONFIGFS_ITEM_ATTR_QMULT(ncm);
+USB_ETHER_OPTS_ATTR_QMULT(ncm);
 
 /* f_ncm_opts_ifname */
-USB_ETHERNET_CONFIGFS_ITEM_ATTR_IFNAME(ncm);
+USB_ETHER_OPTS_ATTR_IFNAME(ncm);
 
 static ssize_t ncm_opts_max_segment_size_show(struct config_item *item,
                                              char *page)
@@ -1655,34 +1672,27 @@ static void ncm_free_inst(struct usb_function_instance *f)
        struct f_ncm_opts *opts;
 
        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);
 }
 
 static struct usb_function_instance *ncm_alloc_inst(void)
 {
-       struct f_ncm_opts *opts;
+       struct usb_function_instance *ret;
        struct usb_os_desc *descs[1];
        char *names[1];
        struct config_group *ncm_interf_group;
 
-       opts = kzalloc(sizeof(*opts), GFP_KERNEL);
+       struct f_ncm_opts *opts __free(kfree) = kzalloc(sizeof(*opts), GFP_KERNEL);
        if (!opts)
                return ERR_PTR(-ENOMEM);
+
+       opts->net = NULL;
        opts->ncm_os_desc.ext_compat_id = opts->ncm_ext_compat_id;
+       gether_setup_opts_default(&opts->net_opts, "usb");
 
        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);
-       }
        opts->max_segment_size = ETH_FRAME_LEN;
        INIT_LIST_HEAD(&opts->ncm_os_desc.ext_prop);
 
@@ -1693,26 +1703,22 @@ static struct usb_function_instance *ncm_alloc_inst(void)
        ncm_interf_group =
                usb_os_desc_prepare_interf_dir(&opts->func_inst.group, 1, descs,
                                               names, THIS_MODULE);
-       if (IS_ERR(ncm_interf_group)) {
-               ncm_free_inst(&opts->func_inst);
+       if (IS_ERR(ncm_interf_group))
                return ERR_CAST(ncm_interf_group);
-       }
        opts->ncm_interf_group = ncm_interf_group;
 
-       return &opts->func_inst;
+       ret = &opts->func_inst;
+       retain_and_null_ptr(opts);
+       return ret;
 }
 
 static void ncm_free(struct usb_function *f)
 {
-       struct f_ncm *ncm;
-       struct f_ncm_opts *opts;
+       struct f_ncm_opts *opts = func_to_ncm_opts(f);
 
-       ncm = func_to_ncm(f);
-       opts = container_of(f->fi, struct f_ncm_opts, func_inst);
-       kfree(ncm);
-       mutex_lock(&opts->lock);
-       opts->refcnt--;
-       mutex_unlock(&opts->lock);
+       scoped_guard(mutex, &opts->lock)
+               opts->refcnt--;
+       kfree(func_to_ncm(f));
 }
 
 static void ncm_unbind(struct usb_configuration *c, struct usb_function *f)
@@ -1736,13 +1742,15 @@ 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);
+
+       ncm->port.ioport = NULL;
+       gether_cleanup(netdev_priv(ncm->netdev));
 }
 
 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);
@@ -1750,22 +1758,12 @@ static struct usb_function *ncm_alloc(struct usb_function_instance *fi)
                return ERR_PTR(-ENOMEM);
 
        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);
-       }
+       scoped_guard(mutex, &opts->lock)
+               opts->refcnt++;
 
        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_ncm.h b/drivers/usb/gadget/function/u_ncm.h
index 49ec095cdb4b6dcb330fd3149b502840312f78ec..d99330fe31e880f636615774d212062952c31e43 100644 (file)
--- a/drivers/usb/gadget/function/u_ncm.h
+++ b/drivers/usb/gadget/function/u_ncm.h
@@ -15,11 +15,13 @@
 
 #include <linux/usb/composite.h>
 
+#include "u_ether.h"
+
 struct f_ncm_opts {
        struct usb_function_instance    func_inst;
        struct net_device               *net;
-       bool                            bound;
 
+       struct gether_opts              net_opts;
        struct config_group             *ncm_interf_group;
        struct usb_os_desc              ncm_os_desc;
        char                            ncm_ext_compat_id[16];