[PATCH net v1 1/2] net: usb: lan78xx: Fix double free issue with interrupt buffer allocation

From: Oleksij Rempel
Date: Sat Nov 16 2024 - 08:06:23 EST


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@xxxxxxxxxxxxx>
Signed-off-by: Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx>
---
drivers/net/usb/lan78xx.c | 29 ++++++++++++++---------------
1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 8adf77e3557e..094a47b8b97e 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -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:
--
2.39.5