From 84811412464d66fcf73661a279aa1d7985166495 Mon Sep 17 00:00:00 2001 From: Prashant Malani Date: Wed, 20 Nov 2019 11:40:21 -0800 Subject: r8152: Re-order napi_disable in rtl8152_close Both rtl_work_func_t() and rtl8152_close() call napi_disable(). Since the two calls aren't protected by a lock, if the close function starts executing before the work function, we can get into a situation where the napi_disable() function is called twice in succession (first by rtl8152_close(), then by set_carrier()). In such a situation, the second call would loop indefinitely, since rtl8152_close() doesn't call napi_enable() to clear the NAPI_STATE_SCHED bit. The rtl8152_close() function in turn issues a cancel_delayed_work_sync(), and so it would wait indefinitely for the rtl_work_func_t() to complete. Since rtl8152_close() is called by a process holding rtnl_lock() which is requested by other processes, this eventually leads to a system deadlock and crash. Re-order the napi_disable() call to occur after the work function disabling and urb cancellation calls are issued. Change-Id: I6ef0b703fc214998a037a68f722f784e1d07815e Reported-by: http://crbug.com/1017928 Signed-off-by: Prashant Malani Signed-off-by: David S. Miller --- drivers/net/usb/r8152.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/net/usb') diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index d4a95b50bda6..4d34c01826f3 100644 --- a/drivers/net/usb/r8152.c +++ b/drivers/net/usb/r8152.c @@ -4283,10 +4283,10 @@ static int rtl8152_close(struct net_device *netdev) unregister_pm_notifier(&tp->pm_notifier); #endif tasklet_disable(&tp->tx_tl); - napi_disable(&tp->napi); clear_bit(WORK_ENABLE, &tp->flags); usb_kill_urb(tp->intr_urb); cancel_delayed_work_sync(&tp->schedule); + napi_disable(&tp->napi); netif_stop_queue(netdev); res = usb_autopm_get_interface(tp->intf); -- cgit v1.2.3 From 5b1d9c17a3e0c16e1c9adf9c8a89f2735cb6dff8 Mon Sep 17 00:00:00 2001 From: Hayes Wang Date: Fri, 22 Nov 2019 16:21:09 +0800 Subject: r8152: avoid to call napi_disable twice Call napi_disable() twice would cause dead lock. There are three situations may result in the issue. 1. rtl8152_pre_reset() and set_carrier() are run at the same time. 2. Call rtl8152_set_tunable() after rtl8152_close(). 3. Call rtl8152_set_ringparam() after rtl8152_close(). For #1, use the same solution as commit 84811412464d ("r8152: Re-order napi_disable in rtl8152_close"). For #2 and #3, add checking the flag of IFF_UP and using napi_disable/napi_enable during mutex. Signed-off-by: Hayes Wang Signed-off-by: David S. Miller --- drivers/net/usb/r8152.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) (limited to 'drivers/net/usb') diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index 4d34c01826f3..b2507c59ba8b 100644 --- a/drivers/net/usb/r8152.c +++ b/drivers/net/usb/r8152.c @@ -4552,10 +4552,10 @@ static int rtl8152_pre_reset(struct usb_interface *intf) netif_stop_queue(netdev); tasklet_disable(&tp->tx_tl); - napi_disable(&tp->napi); clear_bit(WORK_ENABLE, &tp->flags); usb_kill_urb(tp->intr_urb); cancel_delayed_work_sync(&tp->schedule); + napi_disable(&tp->napi); if (netif_carrier_ok(netdev)) { mutex_lock(&tp->control); tp->rtl_ops.disable(tp); @@ -4673,7 +4673,7 @@ static int rtl8152_system_resume(struct r8152 *tp) netif_device_attach(netdev); - if (netif_running(netdev) && netdev->flags & IFF_UP) { + if (netif_running(netdev) && (netdev->flags & IFF_UP)) { tp->rtl_ops.up(tp); netif_carrier_off(netdev); set_bit(WORK_ENABLE, &tp->flags); @@ -5244,9 +5244,15 @@ static int rtl8152_set_tunable(struct net_device *netdev, } if (tp->rx_copybreak != val) { - napi_disable(&tp->napi); - tp->rx_copybreak = val; - napi_enable(&tp->napi); + if (netdev->flags & IFF_UP) { + mutex_lock(&tp->control); + napi_disable(&tp->napi); + tp->rx_copybreak = val; + napi_enable(&tp->napi); + mutex_unlock(&tp->control); + } else { + tp->rx_copybreak = val; + } } break; default: @@ -5274,9 +5280,15 @@ static int rtl8152_set_ringparam(struct net_device *netdev, return -EINVAL; if (tp->rx_pending != ring->rx_pending) { - napi_disable(&tp->napi); - tp->rx_pending = ring->rx_pending; - napi_enable(&tp->napi); + if (netdev->flags & IFF_UP) { + mutex_lock(&tp->control); + napi_disable(&tp->napi); + tp->rx_pending = ring->rx_pending; + napi_enable(&tp->napi); + mutex_unlock(&tp->control); + } else { + tp->rx_pending = ring->rx_pending; + } } return 0; -- cgit v1.2.3