Skip to content

Commit ec35c19

Browse files
Kuen-Han Tsaigregkh
authored andcommitted
usb: gadget: f_ncm: Fix net_device lifecycle with device_move
The network device outlived its parent gadget device during disconnection, resulting in dangling sysfs links and null pointer dereference problems. A prior attempt to solve this by removing SET_NETDEV_DEV entirely [1] was reverted due to power management ordering concerns and a NO-CARRIER regression. A subsequent attempt to defer net_device allocation to bind [2] broke 1:1 mapping between function instance and network device, making it impossible for configfs to report the resolved interface name. This results in a regression where the DHCP server fails on pmOS. Use device_move to reparent the net_device between the gadget device and /sys/devices/virtual/ across bind/unbind cycles. This preserves the network interface across USB reconnection, allowing the DHCP server to retain their binding. Introduce gether_attach_gadget()/gether_detach_gadget() helpers and use __free(detach_gadget) macro to undo attachment on bind failure. The bind_count ensures device_move executes only on the first bind. [1] https://lore.kernel.org/lkml/f2a4f9847617a0929d62025748384092e5f35cce.camel@crapouillou.net/ [2] https://lore.kernel.org/linux-usb/795ea759-7eaf-4f78-81f4-01ffbf2d7961@ixit.cz/ Fixes: 40d133d ("usb: gadget: f_ncm: convert to new function interface with backward compatibility") Cc: stable <stable@kernel.org> Signed-off-by: Kuen-Han Tsai <khtsai@google.com> Link: https://patch.msgid.link/20260309-f-ncm-revert-v2-7-ea2afbc7d9b2@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 3131c1a commit ec35c19

4 files changed

Lines changed: 74 additions & 14 deletions

File tree

drivers/usb/gadget/function/f_ncm.c

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1439,6 +1439,7 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f)
14391439
struct f_ncm_opts *ncm_opts;
14401440

14411441
struct usb_os_desc_table *os_desc_table __free(kfree) = NULL;
1442+
struct net_device *net __free(detach_gadget) = NULL;
14421443
struct usb_request *request __free(free_usb_request) = NULL;
14431444

14441445
if (!can_support_ecm(cdev->gadget))
@@ -1452,18 +1453,19 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f)
14521453
return -ENOMEM;
14531454
}
14541455

1455-
mutex_lock(&ncm_opts->lock);
1456-
gether_set_gadget(ncm_opts->net, cdev->gadget);
1457-
if (!ncm_opts->bound) {
1458-
ncm_opts->net->mtu = (ncm_opts->max_segment_size - ETH_HLEN);
1459-
status = gether_register_netdev(ncm_opts->net);
1460-
}
1461-
mutex_unlock(&ncm_opts->lock);
1462-
1463-
if (status)
1464-
return status;
1465-
1466-
ncm_opts->bound = true;
1456+
scoped_guard(mutex, &ncm_opts->lock)
1457+
if (ncm_opts->bind_count == 0) {
1458+
if (!device_is_registered(&ncm_opts->net->dev)) {
1459+
ncm_opts->net->mtu = (ncm_opts->max_segment_size - ETH_HLEN);
1460+
gether_set_gadget(ncm_opts->net, cdev->gadget);
1461+
status = gether_register_netdev(ncm_opts->net);
1462+
} else
1463+
status = gether_attach_gadget(ncm_opts->net, cdev->gadget);
1464+
1465+
if (status)
1466+
return status;
1467+
net = ncm_opts->net;
1468+
}
14671469

14681470
ncm_string_defs[1].s = ncm->ethaddr;
14691471

@@ -1564,6 +1566,9 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f)
15641566
}
15651567
ncm->notify_req = no_free_ptr(request);
15661568

1569+
ncm_opts->bind_count++;
1570+
retain_and_null_ptr(net);
1571+
15671572
DBG(cdev, "CDC Network: IN/%s OUT/%s NOTIFY/%s\n",
15681573
ncm->port.in_ep->name, ncm->port.out_ep->name,
15691574
ncm->notify->name);
@@ -1655,7 +1660,7 @@ static void ncm_free_inst(struct usb_function_instance *f)
16551660
struct f_ncm_opts *opts;
16561661

16571662
opts = container_of(f, struct f_ncm_opts, func_inst);
1658-
if (opts->bound)
1663+
if (device_is_registered(&opts->net->dev))
16591664
gether_cleanup(netdev_priv(opts->net));
16601665
else
16611666
free_netdev(opts->net);
@@ -1718,9 +1723,12 @@ static void ncm_free(struct usb_function *f)
17181723
static void ncm_unbind(struct usb_configuration *c, struct usb_function *f)
17191724
{
17201725
struct f_ncm *ncm = func_to_ncm(f);
1726+
struct f_ncm_opts *ncm_opts;
17211727

17221728
DBG(c->cdev, "ncm unbind\n");
17231729

1730+
ncm_opts = container_of(f->fi, struct f_ncm_opts, func_inst);
1731+
17241732
hrtimer_cancel(&ncm->task_timer);
17251733

17261734
kfree(f->os_desc_table);
@@ -1736,6 +1744,10 @@ static void ncm_unbind(struct usb_configuration *c, struct usb_function *f)
17361744

17371745
kfree(ncm->notify_req->buf);
17381746
usb_ep_free_request(ncm->notify, ncm->notify_req);
1747+
1748+
ncm_opts->bind_count--;
1749+
if (ncm_opts->bind_count == 0)
1750+
gether_detach_gadget(ncm_opts->net);
17391751
}
17401752

17411753
static struct usb_function *ncm_alloc(struct usb_function_instance *fi)

drivers/usb/gadget/function/u_ether.c

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -897,6 +897,28 @@ void gether_set_gadget(struct net_device *net, struct usb_gadget *g)
897897
}
898898
EXPORT_SYMBOL_GPL(gether_set_gadget);
899899

900+
int gether_attach_gadget(struct net_device *net, struct usb_gadget *g)
901+
{
902+
int ret;
903+
904+
ret = device_move(&net->dev, &g->dev, DPM_ORDER_DEV_AFTER_PARENT);
905+
if (ret)
906+
return ret;
907+
908+
gether_set_gadget(net, g);
909+
return 0;
910+
}
911+
EXPORT_SYMBOL_GPL(gether_attach_gadget);
912+
913+
void gether_detach_gadget(struct net_device *net)
914+
{
915+
struct eth_dev *dev = netdev_priv(net);
916+
917+
device_move(&net->dev, NULL, DPM_ORDER_NONE);
918+
dev->gadget = NULL;
919+
}
920+
EXPORT_SYMBOL_GPL(gether_detach_gadget);
921+
900922
int gether_set_dev_addr(struct net_device *net, const char *dev_addr)
901923
{
902924
struct eth_dev *dev;

drivers/usb/gadget/function/u_ether.h

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,32 @@ static inline struct net_device *gether_setup_default(void)
150150
*/
151151
void gether_set_gadget(struct net_device *net, struct usb_gadget *g);
152152

153+
/**
154+
* gether_attach_gadget - Reparent net_device to the gadget device.
155+
* @net: The network device to reparent.
156+
* @g: The target USB gadget device to parent to.
157+
*
158+
* This function moves the network device to be a child of the USB gadget
159+
* device in the device hierarchy. This is typically done when the function
160+
* is bound to a configuration.
161+
*
162+
* Returns 0 on success, or a negative error code on failure.
163+
*/
164+
int gether_attach_gadget(struct net_device *net, struct usb_gadget *g);
165+
166+
/**
167+
* gether_detach_gadget - Detach net_device from its gadget parent.
168+
* @net: The network device to detach.
169+
*
170+
* This function moves the network device to be a child of the virtual
171+
* devices parent, effectively detaching it from the USB gadget device
172+
* hierarchy. This is typically done when the function is unbound
173+
* from a configuration but the instance is not yet freed.
174+
*/
175+
void gether_detach_gadget(struct net_device *net);
176+
177+
DEFINE_FREE(detach_gadget, struct net_device *, if (_T) gether_detach_gadget(_T))
178+
153179
/**
154180
* gether_set_dev_addr - initialize an ethernet-over-usb link with eth address
155181
* @net: device representing this link

drivers/usb/gadget/function/u_ncm.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
struct f_ncm_opts {
1919
struct usb_function_instance func_inst;
2020
struct net_device *net;
21-
bool bound;
21+
int bind_count;
2222

2323
struct config_group *ncm_interf_group;
2424
struct usb_os_desc ncm_os_desc;

0 commit comments

Comments
 (0)