The intr URB submitted in ems_usb_start() is not anchored, and its
completion handler ems_usb_read_interrupt_callback() resubmits it, so
it stays in flight as long as the interface is up. But unlink_all_urbs()
stops this URB with usb_unlink_urb(), which only initiates an
asynchronous unlink and returns without waiting for the handler.
The handler can therefore still be running while ems_usb_disconnect()
frees its data: it reads the transfer buffer dev->intr_in_buffer, which
is kfree()d there, and dereferences the private context, which is
released via free_candev() together with the network device.
Fix this by stopping the intr URB with usb_kill_urb(), which waits
until the handler has returned, so the frees in ems_usb_disconnect()
happen strictly after the last callback.
The handler treats the -ENOENT completion of a killed URB as terminal and
does not take RTNL or any sleeping lock, so the resubmit loop is cut and
no RTNL deadlock occurs.
This issue was found by an in-house static analysis tool.
Fixes: 702171adeed3 ("ems_usb: Added support for EMS CPC-USB/ARM7 CAN/USB interface")
Cc: stable@vger.kernel.org
Co-developed-by: Song Li <songl@zju.edu.cn>
Signed-off-by: Song Li <songl@zju.edu.cn>
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
drivers/net/can/usb/ems_usb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/can/usb/ems_usb.c b/drivers/net/can/usb/ems_usb.c
index 9b25dda..9ffb195 100644
--- a/drivers/net/can/usb/ems_usb.c
+++ b/drivers/net/can/usb/ems_usb.c
@@ -705,7 +705,7 @@ static void unlink_all_urbs(struct ems_usb *dev)
{
int i;
- usb_unlink_urb(dev->intr_urb);
+ usb_kill_urb(dev->intr_urb);
usb_kill_anchored_urbs(&dev->rx_submitted);