[PATCH] media: usb: siano: don't set URB_FREE_BUFFER flag

Tetsuo Handa posted 1 patch 1 month, 4 weeks ago
drivers/media/usb/siano/smsusb.c | 1 -
1 file changed, 1 deletion(-)
[PATCH] media: usb: siano: don't set URB_FREE_BUFFER flag
Posted by Tetsuo Handa 1 month, 4 weeks ago
syzbot is reporting invalid free at usb_free_urb(), for

  smscore_register_device() allocates all buffers at once as an array

  smscore_createbuffer() maps each element in the array to cb->p

  usb_fill_bulk_urb() assigns urb->transfer_buffer using cb->p
  which may point to a non-head element in the array

  URB_FREE_BUFFER causes usb_free_urb() to free urb->transfer_buffer
  which may point to a non-head element in the array

The urb->transfer_buffer must point to an address returned by kmalloc()
family if URB_FREE_BUFFER flag is set. But since the urb->transfer_buffer
allocation strategy for this module is to allocate buffers upon device
registration and free buffers upon device unregistration, we should
avoid setting URB_FREE_BUFFER flag. Otherwise, double free or invalid
free will happen.

Reported-by: syzbot+b466336413a1fba398a5@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=b466336413a1fba398a5
Fixes: 564246fd3ff4 ("media: siano: Fix coherent memory allocation failure on arm64")
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
I found that the same change was proposed at
https://lore.kernel.org/all/20250522140048.2811356-1-n.zhandarovich@fintech.ru/T/
after I wrote this patch.
If nobody is using this module, we should consider removing this module?

 drivers/media/usb/siano/smsusb.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/usb/siano/smsusb.c b/drivers/media/usb/siano/smsusb.c
index 0fdc2e0950b7..8140dc0c8b7d 100644
--- a/drivers/media/usb/siano/smsusb.c
+++ b/drivers/media/usb/siano/smsusb.c
@@ -168,7 +168,6 @@ static int smsusb_submit_urb(struct smsusb_device_t *dev,
 		smsusb_onresponse,
 		surb
 	);
-	surb->urb->transfer_flags |= URB_FREE_BUFFER;
 
 	return usb_submit_urb(surb->urb, GFP_ATOMIC);
 }
-- 
2.47.3
Re: [PATCH] media: usb: siano: don't set URB_FREE_BUFFER flag
Posted by Tomoki Sekiyama 1 month, 4 weeks ago
Hi,
I have tested your patch with PLEX PX-S1UD V2.0 (siano USB tuner) and
confirmed that it fixes the following warnings
originally shown when the driver is unloaded or the tuner is unplugged
WITHOUT your patch.

[  109.352681] WARNING: mm/slub.c:6914 at
free_large_kmalloc+0x71/0xc0, CPU#3: modprobe/18686
...
[  109.352856] Call Trace:
[  109.352858]  <TASK>
[  109.352862]  usb_free_urb+0x4e/0x60
[  109.352868]  smsusb_term_device+0x5d/0xd0 [smsusb]
[  109.352875]  usb_unbind_interface+0x9d/0x2c0
[  109.352881]  device_release_driver_internal+0x19e/0x200
[  109.352887]  driver_detach+0x48/0x90
[  109.352890]  bus_remove_driver+0x6d/0x100
[  109.352894]  usb_deregister+0x64/0xe7
[  109.352899]  __do_sys_delete_module.isra.0+0x1b6/0x2e0
[  109.352906]  do_syscall_64+0x7e/0x690
...
[  109.353204] page dumped because: Not a kmalloc allocation


> If nobody is using this module, we should consider removing this module?
At least I have a device for this module and compatible devices like
MyGica S270 seems still available on Amazon.

Tested-by: Tomoki Sekiyama <tomoki.sekiyama@gmail.com>

Thanks!

2026年4月17日(金) 23:30 Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>:
>
> syzbot is reporting invalid free at usb_free_urb(), for
>
>   smscore_register_device() allocates all buffers at once as an array
>
>   smscore_createbuffer() maps each element in the array to cb->p
>
>   usb_fill_bulk_urb() assigns urb->transfer_buffer using cb->p
>   which may point to a non-head element in the array
>
>   URB_FREE_BUFFER causes usb_free_urb() to free urb->transfer_buffer
>   which may point to a non-head element in the array
>
> The urb->transfer_buffer must point to an address returned by kmalloc()
> family if URB_FREE_BUFFER flag is set. But since the urb->transfer_buffer
> allocation strategy for this module is to allocate buffers upon device
> registration and free buffers upon device unregistration, we should
> avoid setting URB_FREE_BUFFER flag. Otherwise, double free or invalid
> free will happen.
>
> Reported-by: syzbot+b466336413a1fba398a5@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=b466336413a1fba398a5
> Fixes: 564246fd3ff4 ("media: siano: Fix coherent memory allocation failure on arm64")
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---
> I found that the same change was proposed at
> https://lore.kernel.org/all/20250522140048.2811356-1-n.zhandarovich@fintech.ru/T/
> after I wrote this patch.
> If nobody is using this module, we should consider removing this module?
>
>  drivers/media/usb/siano/smsusb.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/media/usb/siano/smsusb.c b/drivers/media/usb/siano/smsusb.c
> index 0fdc2e0950b7..8140dc0c8b7d 100644
> --- a/drivers/media/usb/siano/smsusb.c
> +++ b/drivers/media/usb/siano/smsusb.c
> @@ -168,7 +168,6 @@ static int smsusb_submit_urb(struct smsusb_device_t *dev,
>                 smsusb_onresponse,
>                 surb
>         );
> -       surb->urb->transfer_flags |= URB_FREE_BUFFER;
>
>         return usb_submit_urb(surb->urb, GFP_ATOMIC);
>  }
> --
> 2.47.3
>