| Message ID | 20210616024833.2761919-1-mudongliangabcd@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 56b786d86694e079d8aad9b314e015cd4ac02a3d |
| Headers | show |
| Series | [v2] net: usb: fix possible use-after-free in smsc75xx_bind | expand |
Hello: This patch was applied to netdev/net.git (refs/heads/master): On Wed, 16 Jun 2021 10:48:33 +0800 you wrote: > The commit 46a8b29c6306 ("net: usb: fix memory leak in smsc75xx_bind") > fails to clean up the work scheduled in smsc75xx_reset-> > smsc75xx_set_multicast, which leads to use-after-free if the work is > scheduled to start after the deallocation. In addition, this patch > also removes a dangling pointer - dev->data[0]. > > This patch calls cancel_work_sync to cancel the scheduled work and set > the dangling pointer to NULL. > > [...] Here is the summary with links: - [v2] net: usb: fix possible use-after-free in smsc75xx_bind https://git.kernel.org/netdev/net/c/56b786d86694 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c index b286993da67c..13141dbfa3a8 100644 --- a/drivers/net/usb/smsc75xx.c +++ b/drivers/net/usb/smsc75xx.c @@ -1483,7 +1483,7 @@ static int smsc75xx_bind(struct usbnet *dev, struct usb_interface *intf) ret = smsc75xx_wait_ready(dev, 0); if (ret < 0) { netdev_warn(dev->net, "device not ready in smsc75xx_bind\n"); - goto err; + goto free_pdata; } smsc75xx_init_mac_address(dev); @@ -1492,7 +1492,7 @@ static int smsc75xx_bind(struct usbnet *dev, struct usb_interface *intf) ret = smsc75xx_reset(dev); if (ret < 0) { netdev_warn(dev->net, "smsc75xx_reset error %d\n", ret); - goto err; + goto cancel_work; } dev->net->netdev_ops = &smsc75xx_netdev_ops; @@ -1503,8 +1503,11 @@ static int smsc75xx_bind(struct usbnet *dev, struct usb_interface *intf) dev->net->max_mtu = MAX_SINGLE_PACKET_SIZE; return 0; -err: +cancel_work: + cancel_work_sync(&pdata->set_multicast); +free_pdata: kfree(pdata); + dev->data[0] = 0; return ret; } @@ -1515,7 +1518,6 @@ static void smsc75xx_unbind(struct usbnet *dev, struct usb_interface *intf) cancel_work_sync(&pdata->set_multicast); netif_dbg(dev, ifdown, dev->net, "free pdata\n"); kfree(pdata); - pdata = NULL; dev->data[0] = 0; } }
The commit 46a8b29c6306 ("net: usb: fix memory leak in smsc75xx_bind") fails to clean up the work scheduled in smsc75xx_reset-> smsc75xx_set_multicast, which leads to use-after-free if the work is scheduled to start after the deallocation. In addition, this patch also removes a dangling pointer - dev->data[0]. This patch calls cancel_work_sync to cancel the scheduled work and set the dangling pointer to NULL. Fixes: 46a8b29c6306 ("net: usb: fix memory leak in smsc75xx_bind") Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com> --- v1->v2: split the err label into two labels - cancel_work and free_data according to Pavel Skripkin; remove "pdata = NULL" according to gregkh drivers/net/usb/smsc75xx.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)