[PATCH] usb: gadget: raw_gadget: fix double free in raw_release

cuiyudong posted 1 patch 1 week ago
There is a newer version of this series
drivers/usb/gadget/legacy/raw_gadget.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
[PATCH] usb: gadget: raw_gadget: fix double free in raw_release
Posted by cuiyudong 1 week ago
raw_release() had duplicate kref_put() which caused KASAN double-free.
The extra put inside the unregister block is removed to balance refcount.

BUG: KASAN: double-free in dev_free+0x424/0x740
Fixes: f2c2e717642c ("usb: gadget: add raw-gadget interface")
Tested-by: syzbot+25612fe5ab3dcafc3aab@syzkaller.appspotmail.com
Reported-by: syzbot+25612fe5ab3dcafc3aab@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/69c401ad.a70a0220.23629d.0000.GAE@google.com/
Signed-off-by: cuiyudong <cuiyudong@kylinos.cn>
---
 drivers/usb/gadget/legacy/raw_gadget.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/legacy/raw_gadget.c b/drivers/usb/gadget/legacy/raw_gadget.c
index 4febf8dac7ca..a1fd3fdf1323 100644
--- a/drivers/usb/gadget/legacy/raw_gadget.c
+++ b/drivers/usb/gadget/legacy/raw_gadget.c
@@ -465,12 +465,10 @@ static int raw_release(struct inode *inode, struct file *fd)
 			dev_err(dev->dev,
 				"usb_gadget_unregister_driver() failed with %d\n",
 				ret);
-		/* Matches kref_get() in raw_ioctl_run(). */
-		kref_put(&dev->count, dev_free);
 	}
 
 out_put:
-	/* Matches dev_new() in raw_open(). */
+	/* Matches dev_new() in raw_open() and kref_get() in raw_ioctl_run(). */
 	kref_put(&dev->count, dev_free);
 	return ret;
 }
-- 
2.25.1
Re: [PATCH] usb: gadget: raw_gadget: fix double free in raw_release
Posted by Andrey Konovalov 1 week ago
On Thu, Mar 26, 2026 at 8:45 AM cuiyudong <cuiyudong@kylinos.cn> wrote:
>
> raw_release() had duplicate kref_put() which caused KASAN double-free.
> The extra put inside the unregister block is removed to balance refcount.

This patch is missing the details that explain what happens and why
this fix makes sense (and I cannot tell that it does).

Why is the removed kref_put() extra?

Why did we get a KASAN report instead of a refcount warning, if
kref_put() is the problem?

>
> BUG: KASAN: double-free in dev_free+0x424/0x740
> Fixes: f2c2e717642c ("usb: gadget: add raw-gadget interface")
> Tested-by: syzbot+25612fe5ab3dcafc3aab@syzkaller.appspotmail.com

Why is there a Tested-by tag here? I don't see a patch testing request
on the syzbot page.

Moreover, there's no reproducer for this bug, so syzbot could not have
tested this fix.

> Reported-by: syzbot+25612fe5ab3dcafc3aab@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/all/69c401ad.a70a0220.23629d.0000.GAE@google.com/
> Signed-off-by: cuiyudong <cuiyudong@kylinos.cn>
> ---
>  drivers/usb/gadget/legacy/raw_gadget.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/usb/gadget/legacy/raw_gadget.c b/drivers/usb/gadget/legacy/raw_gadget.c
> index 4febf8dac7ca..a1fd3fdf1323 100644
> --- a/drivers/usb/gadget/legacy/raw_gadget.c
> +++ b/drivers/usb/gadget/legacy/raw_gadget.c
> @@ -465,12 +465,10 @@ static int raw_release(struct inode *inode, struct file *fd)
>                         dev_err(dev->dev,
>                                 "usb_gadget_unregister_driver() failed with %d\n",
>                                 ret);
> -               /* Matches kref_get() in raw_ioctl_run(). */
> -               kref_put(&dev->count, dev_free);
>         }
>
>  out_put:
> -       /* Matches dev_new() in raw_open(). */
> +       /* Matches dev_new() in raw_open() and kref_get() in raw_ioctl_run(). */
>         kref_put(&dev->count, dev_free);
>         return ret;
>  }
> --
> 2.25.1
>