]> www.infradead.org Git - users/willy/pagecache.git/commitdiff
net: usb: lan78xx: Fix double free issue with interrupt buffer allocation
authorOleksij Rempel <o.rempel@pengutronix.de>
Sat, 16 Nov 2024 13:05:57 +0000 (14:05 +0100)
committerJakub Kicinski <kuba@kernel.org>
Mon, 25 Nov 2024 00:28:57 +0000 (16:28 -0800)
In lan78xx_probe(), the buffer `buf` was being freed twice: once
implicitly through `usb_free_urb(dev->urb_intr)` with the
`URB_FREE_BUFFER` flag and again explicitly by `kfree(buf)`. This caused
a double free issue.

To resolve this, reordered `kmalloc()` and `usb_alloc_urb()` calls to
simplify the initialization sequence and removed the redundant
`kfree(buf)`.  Now, `buf` is allocated after `usb_alloc_urb()`, ensuring
it is correctly managed by  `usb_fill_int_urb()` and freed by
`usb_free_urb()` as intended.

Fixes: a6df95cae40b ("lan78xx: Fix memory allocation bug")
Cc: John Efstathiades <john.efstathiades@pebblebay.com>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Link: https://patch.msgid.link/20241116130558.1352230-1-o.rempel@pengutronix.de
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
drivers/net/usb/lan78xx.c

index 8adf77e3557e7a49ba340de53ab71db027db048c..094a47b8b97eb2136d673282078ae4baa17a6cdc 100644 (file)
@@ -4414,29 +4414,30 @@ static int lan78xx_probe(struct usb_interface *intf,
 
        period = ep_intr->desc.bInterval;
        maxp = usb_maxpacket(dev->udev, dev->pipe_intr);
-       buf = kmalloc(maxp, GFP_KERNEL);
-       if (!buf) {
+
+       dev->urb_intr = usb_alloc_urb(0, GFP_KERNEL);
+       if (!dev->urb_intr) {
                ret = -ENOMEM;
                goto out5;
        }
 
-       dev->urb_intr = usb_alloc_urb(0, GFP_KERNEL);
-       if (!dev->urb_intr) {
+       buf = kmalloc(maxp, GFP_KERNEL);
+       if (!buf) {
                ret = -ENOMEM;
-               goto out6;
-       } else {
-               usb_fill_int_urb(dev->urb_intr, dev->udev,
-                                dev->pipe_intr, buf, maxp,
-                                intr_complete, dev, period);
-               dev->urb_intr->transfer_flags |= URB_FREE_BUFFER;
+               goto free_urbs;
        }
 
+       usb_fill_int_urb(dev->urb_intr, dev->udev,
+                        dev->pipe_intr, buf, maxp,
+                        intr_complete, dev, period);
+       dev->urb_intr->transfer_flags |= URB_FREE_BUFFER;
+
        dev->maxpacket = usb_maxpacket(dev->udev, dev->pipe_out);
 
        /* Reject broken descriptors. */
        if (dev->maxpacket == 0) {
                ret = -ENODEV;
-               goto out6;
+               goto free_urbs;
        }
 
        /* driver requires remote-wakeup capability during autosuspend. */
@@ -4444,7 +4445,7 @@ static int lan78xx_probe(struct usb_interface *intf,
 
        ret = lan78xx_phy_init(dev);
        if (ret < 0)
-               goto out7;
+               goto free_urbs;
 
        ret = register_netdev(netdev);
        if (ret != 0) {
@@ -4466,10 +4467,8 @@ static int lan78xx_probe(struct usb_interface *intf,
 
 out8:
        phy_disconnect(netdev->phydev);
-out7:
+free_urbs:
        usb_free_urb(dev->urb_intr);
-out6:
-       kfree(buf);
 out5:
        lan78xx_unbind(dev, intf);
 out4: