usb: gadget: f_subset: Fix net_device lifecycle with device_move

The net_device is allocated during function instance creation and
registered during the bind phase with the gadget device as its sysfs
parent. When the function unbinds, the parent device is destroyed, but
the net_device survives, resulting in dangling sysfs symlinks:

  console:/ # ls -l /sys/class/net/usb0
  lrwxrwxrwx ... /sys/class/net/usb0 ->
  /sys/devices/platform/.../gadget.0/net/usb0
  console:/ # ls -l /sys/devices/platform/.../gadget.0/net/usb0
  ls: .../gadget.0/net/usb0: No such file or directory

Use device_move() to reparent the net_device between the gadget device
tree and /sys/devices/virtual across bind and unbind cycles. During the
final unbind, calling device_move(NULL) moves the net_device to the
virtual device tree before the gadget device is destroyed. On rebinding,
device_move() reparents the device back under the new gadget, ensuring
proper sysfs topology and power management ordering.

To maintain compatibility with legacy composite drivers (e.g., multi.c),
the bound flag is used to indicate whether the network device is shared
and pre-registered during the legacy driver's bind phase.

Fixes: 8cedba7c73af ("usb: gadget: f_subset: convert to new function interface with backward compatibility")
Cc: stable@vger.kernel.org
Signed-off-by: Kuen-Han Tsai <khtsai@google.com>
Link: https://patch.msgid.link/20260320-usb-net-lifecycle-v1-6-4886b578161b@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/usb/gadget/function/f_subset.c b/drivers/usb/gadget/function/f_subset.c
index 74dc6da5c7674a222e754c95b7db9a9838605f00..6e3265b8a3a0354b2e6ea52b5c319a52c6041c7c 100644 (file)
--- a/drivers/usb/gadget/function/f_subset.c
+++ b/drivers/usb/gadget/function/f_subset.c
@@ -299,25 +299,22 @@ geth_bind(struct usb_configuration *c, struct usb_function *f)
        struct usb_ep           *ep;
 
        struct f_gether_opts    *gether_opts;
+       struct net_device       *net __free(detach_gadget) = NULL;
 
        gether_opts = container_of(f->fi, struct f_gether_opts, func_inst);
 
-       /*
-        * 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 gether_opts->bound access
-        */
-       if (!gether_opts->bound) {
-               mutex_lock(&gether_opts->lock);
-               gether_set_gadget(gether_opts->net, cdev->gadget);
-               status = gether_register_netdev(gether_opts->net);
-               mutex_unlock(&gether_opts->lock);
-               if (status)
-                       return status;
-               gether_opts->bound = true;
-       }
+       scoped_guard(mutex, &gether_opts->lock)
+               if (gether_opts->bind_count == 0 && !gether_opts->bound) {
+                       if (!device_is_registered(&gether_opts->net->dev)) {
+                               gether_set_gadget(gether_opts->net, cdev->gadget);
+                               status = gether_register_netdev(gether_opts->net);
+                       } else
+                               status = gether_attach_gadget(gether_opts->net, cdev->gadget);
+
+                       if (status)
+                               return status;
+                       net = gether_opts->net;
+               }
 
        us = usb_gstrings_attach(cdev, geth_strings,
                                 ARRAY_SIZE(geth_string_defs));
@@ -330,20 +327,18 @@ geth_bind(struct usb_configuration *c, struct usb_function *f)
        /* allocate instance-specific interface IDs */
        status = usb_interface_id(c, f);
        if (status < 0)
-               goto fail;
+               return status;
        subset_data_intf.bInterfaceNumber = status;
 
-       status = -ENODEV;
-
        /* allocate instance-specific endpoints */
        ep = usb_ep_autoconfig(cdev->gadget, &fs_subset_in_desc);
        if (!ep)
-               goto fail;
+               return -ENODEV;
        geth->port.in_ep = ep;
 
        ep = usb_ep_autoconfig(cdev->gadget, &fs_subset_out_desc);
        if (!ep)
-               goto fail;
+               return -ENODEV;
        geth->port.out_ep = ep;
 
        /* support all relevant hardware speeds... we expect that when
@@ -361,21 +356,19 @@ geth_bind(struct usb_configuration *c, struct usb_function *f)
        status = usb_assign_descriptors(f, fs_eth_function, hs_eth_function,
                        ss_eth_function, ss_eth_function);
        if (status)
-               goto fail;
+               return status;
 
        /* NOTE:  all that is done without knowing or caring about
         * the network link ... which is unavailable to this code
         * until we're activated via set_alt().
         */
 
+       gether_opts->bind_count++;
+       retain_and_null_ptr(net);
+
        DBG(cdev, "CDC Subset: IN/%s OUT/%s\n",
                        geth->port.in_ep->name, geth->port.out_ep->name);
        return 0;
-
-fail:
-       ERROR(cdev, "%s: can't bind, err %d\n", f->name, status);
-
-       return status;
 }
 
 static inline struct f_gether_opts *to_f_gether_opts(struct config_item *item)
@@ -418,7 +411,7 @@ static void geth_free_inst(struct usb_function_instance *f)
        struct f_gether_opts *opts;
 
        opts = container_of(f, struct f_gether_opts, func_inst);
-       if (opts->bound)
+       if (device_is_registered(&opts->net->dev))
                gether_cleanup(netdev_priv(opts->net));
        else
                free_netdev(opts->net);
@@ -462,8 +455,16 @@ static void geth_free(struct usb_function *f)
 
 static void geth_unbind(struct usb_configuration *c, struct usb_function *f)
 {
+       struct f_gether_opts *opts;
+
+       opts = container_of(f->fi, struct f_gether_opts, func_inst);
+
        geth_string_defs[0].id = 0;
        usb_free_all_descriptors(f);
+
+       opts->bind_count--;
+       if (opts->bind_count == 0 && !opts->bound)
+               gether_detach_gadget(opts->net);
 }
 
 static struct usb_function *geth_alloc(struct usb_function_instance *fi)

diff --git a/drivers/usb/gadget/function/u_gether.h b/drivers/usb/gadget/function/u_gether.h
index 2f7a373ed4496a61d76337adbcd2a22900d67b39..e7b6b51f69c12ab16d3240f18b69ea8f37a0d850 100644 (file)
--- a/drivers/usb/gadget/function/u_gether.h
+++ b/drivers/usb/gadget/function/u_gether.h
@@ -15,17 +15,25 @@
 
 #include <linux/usb/composite.h>
 
+/**
+ * struct f_gether_opts - subset function options
+ * @func_inst: USB function instance.
+ * @net: The net_device associated with the subset function.
+ * @bound: True if the net_device is shared and pre-registered during the
+ *         legacy composite driver's bind phase (e.g., multi.c). If false,
+ *         the subset function will register the net_device during its own
+ *         bind phase.
+ * @bind_count: Tracks the number of configurations the subset function is
+ *              bound to, preventing double-registration of the @net device.
+ * @lock: Protects the data from concurrent access by configfs read/write
+ *        and create symlink/remove symlink operations.
+ * @refcnt: Reference counter for the function instance.
+ */
 struct f_gether_opts {
        struct usb_function_instance    func_inst;
        struct net_device               *net;
        bool                            bound;
-
-       /*
-        * Read/write access to configfs attributes is handled by configfs.
-        *
-        * This is to protect the data from concurrent access by read/write
-        * and create symlink/remove symlink.
-        */
+       int                             bind_count;
        struct mutex                    lock;
        int                             refcnt;
 };