Skip to content

Commit e62adae

Browse files
diandersdavem330
authored andcommitted
r8152: Hold the rtnl_lock for all of reset
As of commit d9962b0 ("r8152: Block future register access if register access fails") there is a race condition that can happen between the USB device reset thread and napi_enable() (not) getting called during rtl8152_open(). Specifically: * While rtl8152_open() is running we get a register access error that's _not_ -ENODEV and queue up a USB reset. * rtl8152_open() exits before calling napi_enable() due to any reason (including usb_submit_urb() returning an error). In that case: * Since the USB reset is perform in a separate thread asynchronously, it can run at anytime USB device lock is not held - even before rtl8152_open() has exited with an error and caused __dev_open() to clear the __LINK_STATE_START bit. * The rtl8152_pre_reset() will notice that the netif_running() returns true (since __LINK_STATE_START wasn't cleared) so it won't exit early. * rtl8152_pre_reset() will then hang in napi_disable() because napi_enable() was never called. We can fix the race by making sure that the r8152 reset routines don't run at the same time as we're opening the device. Specifically we need the reset routines in their entirety rely on the return value of netif_running(). The only way to reliably depend on that is for them to hold the rntl_lock() mutex for the duration of reset. Grabbing the rntl_lock() mutex for the duration of reset seems like a long time, but reset is not expected to be common and the rtnl_lock() mutex is already held for long durations since the core grabs it around the open/close calls. Fixes: d9962b0 ("r8152: Block future register access if register access fails") Reviewed-by: Grant Grundler <[email protected]> Signed-off-by: Douglas Anderson <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent 4b0768b commit e62adae

File tree

1 file changed

+7
-6
lines changed

1 file changed

+7
-6
lines changed

drivers/net/usb/r8152.c

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8397,6 +8397,8 @@ static int rtl8152_pre_reset(struct usb_interface *intf)
83978397
struct r8152 *tp = usb_get_intfdata(intf);
83988398
struct net_device *netdev;
83998399

8400+
rtnl_lock();
8401+
84008402
if (!tp || !test_bit(PROBED_WITH_NO_ERRORS, &tp->flags))
84018403
return 0;
84028404

@@ -8428,20 +8430,17 @@ static int rtl8152_post_reset(struct usb_interface *intf)
84288430
struct sockaddr sa;
84298431

84308432
if (!tp || !test_bit(PROBED_WITH_NO_ERRORS, &tp->flags))
8431-
return 0;
8433+
goto exit;
84328434

84338435
rtl_set_accessible(tp);
84348436

84358437
/* reset the MAC address in case of policy change */
8436-
if (determine_ethernet_addr(tp, &sa) >= 0) {
8437-
rtnl_lock();
8438+
if (determine_ethernet_addr(tp, &sa) >= 0)
84388439
dev_set_mac_address (tp->netdev, &sa, NULL);
8439-
rtnl_unlock();
8440-
}
84418440

84428441
netdev = tp->netdev;
84438442
if (!netif_running(netdev))
8444-
return 0;
8443+
goto exit;
84458444

84468445
set_bit(WORK_ENABLE, &tp->flags);
84478446
if (netif_carrier_ok(netdev)) {
@@ -8460,6 +8459,8 @@ static int rtl8152_post_reset(struct usb_interface *intf)
84608459
if (!list_empty(&tp->rx_done))
84618460
napi_schedule(&tp->napi);
84628461

8462+
exit:
8463+
rtnl_unlock();
84638464
return 0;
84648465
}
84658466

0 commit comments

Comments
 (0)