Re: [PATCH] usb: mon: Fix slab-out-of-bounds in mon_bin_event due to unsafe URB transfer_buffer access

[Lecomte, Arnaud]

Hi Alan, thanks for your reply.

Your point raises an important question for me: Is there a specific 
reason why we don’t have
 a synchronization mechanism in place to protect the URB's transfer 
buffer ?


Arnaud


On 21/07/2025 02:29, Alan Stern wrote:
> On Sun, Jul 20, 2025 at 09:00:57PM +0100, Arnaud Lecomte wrote:
> > The syzkaller fuzzer uncovered a kernel slab-out-of-bounds
> >   write in the USB monitoring subsystem (mon_bin) when handling
> >   a malformed URB (USB Request Block) with the following properties:
> >   - transfer_buffer_length = 0xffff
> >   - actual_length = 0x0 (no data transferred)
> >   - number_of_packets = 0x0 (non-isochronous transfer)
> This kind of problem is fixed not by changing the way mon_bin reacts to
> malformed URBs, but rather by correcting the code that creates the URBs
> in the first place so that they won't be malformed.
>
> > When reaching the mon_copy_to_buff function,
> >   we will try to copy into the mon rp bin with the following parameters:
> > off=0xcc0, from=0xffff8880246df5e1 "", length=0xf000
> >
> > At the first iteration, the step_len is 0x340 and it is during the mem_cpy
> > that the slab-out-of-bounds happens.
> > As step_len < transfer_buffer_length, we can deduce that it is related
> >   to an issue with the transfer_buffer being invalid.
> > The patch proposes a safe access to the kernel
> >   kernel buffer urb->transfer_buffer with `copy_from_kernel_nofault`.
> >
> > Reported-by: syzbot+86b6d7c8bcc66747c505@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Fixes: 6f23ee1fefdc1 ("USB: add binary API to usbmon")
> > Closes: https://syzkaller.appspot.com/bug?extid=86b6d7c8bcc66747c505
> > Tested-by: syzbot+86b6d7c8bcc66747c505@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Signed-off-by: Arnaud Lecomte <contact@xxxxxxxxxxxxxx>
> > ---
> This is unnecessary.  The underlying cause of the problem was fixed by
> commit 0d0777ccaa2d ("HID: core: ensure __hid_request reserves the
> report ID as the first byte") in the HID tree.
>
> Alan Stern