Re: WARNING in usb_free_urb

[Vlastimil Babka (SUSE)]

On 6/29/26 09:20, Michal Pecio wrote:
> On Mon, 29 Jun 2026 08:27:48 +0200, Vlastimil Babka (SUSE) wrote:
>> On 6/26/26 23:27, sanan.hasanou@xxxxxxxxx wrote:
>> > Good day, dear maintainers,
>> > 
>> > We found a bug using a modified version of syzkaller.  
>> 
>> Subject says "usb_free_urb" but you only CC'd slab maintainers, where slab
>> slab is most likely a victim here of e.g. double kfree() or a kfree() of
>> otherwise broken pointer.
>> 
>> Ccing USB and EM28XX maintainers. But they can feel free to ignore this per
>> the next point.
>> 
>> > Kernel Branch: 7.0-rc1  
>> 
>> Why use such a version for fuzzing? rc1 will have many bugs that are
>> already fixed in 7.0 final. And it's not even latest, 7.1 was
>> released 2 weeks ago too.
> 
> To be fair, em28xx had no changes since 2024 until 7.1-rc1, so the bug
> must be present in various stable releases and likely in mainline too.

OK I didn't check that, but in general my comment stands.

>> > WARNING: mm/slub.c:6352 at free_large_kmalloc+0xb3/0x160 mm/slub.c:6352, CPU#1: kworker/1:4/12317  
>> 
>> A kfree() was attempted on a pointer that's neither from a slab page nor a
>> large kmalloc page. Might be double free or corrupted.
>> 
>> > Call Trace:
>> >  <TASK>
>> >  kfree+0xae/0x630 mm/slub.c:6437
>> >  urb_destroy drivers/usb/core/urb.c:25 [inline]  
>> 
>> static void urb_destroy(struct kref *kref)
>> {
>>         struct urb *urb = to_urb(kref);
>> 
>>         if (urb->transfer_flags & URB_FREE_BUFFER)
>>                 kfree(urb->transfer_buffer);  <--- this one
>> 
>>         kfree(urb);
>> }
>> 
>> >  kref_put include/linux/kref.h:65 [inline]
>> >  usb_free_urb+0xd1/0x120 drivers/usb/core/urb.c:96  
>> 
>> USB layer itself is likely also not the root cause.
>> 
>> >  em28xx_uninit_usb_xfer+0x165/0x310 drivers/media/usb/em28xx/em28xx-core.c:833
>> >  em28xx_alloc_urbs+0xf2a/0x1130 drivers/media/usb/em28xx/em28xx-core.c:-1
>> >  em28xx_dvb_init+0x2b0/0x4a20 drivers/media/usb/em28xx/em28xx-dvb.c:-1
>> >  em28xx_init_extension+0x121/0x1d0 drivers/media/usb/em28xx/em28xx-core.c:1117  
>> 
>> So it might be this driver doing something wrong?
> 
> Yes, it is. 

Cool :)

>         /* allocate urbs and transfer buffers */
>         for (i = 0; i < usb_bufs->num_bufs; i++) {
>                 urb = usb_alloc_urb(usb_bufs->num_packets, GFP_KERNEL);
>                 if (!urb) {
>                         em28xx_uninit_usb_xfer(dev, mode);
>                         return -ENOMEM;
>                 }
>                 usb_bufs->urb[i] = urb;
> 
>                 usb_bufs->buf[i] = kzalloc(sb_size, GFP_KERNEL);
>                 if (!usb_bufs->buf[i]) {
>                         for (i--; i >= 0; i--)
>                                 kfree(usb_bufs->buf[i]);
> 
>                         em28xx_uninit_usb_xfer(dev, mode);
>                         return -ENOMEM;
>                 }
> 
>                 urb->transfer_flags = URB_FREE_BUFFER;
> 
> If buf[i] allocation fails, all previous buffers are freed and then all
> previous URBs are destroyed. But they already have the URB_FREE_BUFFER
> flag set, which causes a double free as shown above.
> 
> The free(buf[i]) loop should simply be removed. It was mistakenly added
> by d571b592c6206, then a26efd1961a18 recognized the double free but
> attempted to fix it only by changing the order of freeing. Sent from
> .edu domain, so probably an automatic static analyzer fix...
> 
> Regards,
> Michal