Re: WARNING in usb_free_urb

[Michal Pecio <michal.pecio@gmail.com>]

On Mon, 29 Jun 2026 08:27:48 +0200, Vlastimil Babka (SUSE) wrote:
> On 6/26/26 23:27, sanan.hasanou@gmail.com 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.

> > 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. 

        /* 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