On Tue, Apr 14, 2020 at 10:45 PM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > > On Tue, 14 Apr 2020, Andrey Konovalov wrote: > > > On Tue, Apr 14, 2020 at 5:50 PM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > > > > This has got a couple of problems. > > > > > > Firstly, for control URBs it doesn't check urb->setup_packet, which > > > should always be initialized regardless of the direction because it > > > always gets sent to the device. > > > > Ah, yes, we actually had an info-leak report for a bug in the sound > > subsystem that was leaking data this way, which was misattributed to > > raw-gadget. This makes sense to add and seems quite easy. > > > > > Secondly, some URBs use scatter-gather transfers, and they don't always > > > store the buffer address in urb->transfer_buffer (indeed, sometimes the > > > buffer is located outside of the kernel's address map). Instead they > > > use urb->sg and urb->num_sgs. To get an idea for how it all works, > > > look at usb_hcd_map_urb_for_dma() in hcd.c. > > > > Oh, this is something we need to look into. > > > > > Thirdly, the information we get back from the device doesn't always > > > cover the entire transfer buffer; sometimes the device sends less data > > > than we asked for. Perhaps you don't care very much about this case. > > > > So you mean that we should look at actual_length rather than > > transfer_buffer_length when initializing memory that comes from the > > device? > > Yes, in theory. Isochronous transfers are difficult, because the data > doesn't have to be contiguous in memory. If you want to handle them > properly, you have to loop through the entries in the > urb->iso_frame_desc[] array and treat them individually. > Thanks! I'll take a closer look and will probably ask you more questions, as we're extensively fuzzing USB on syzbot. However for the first stage of upstreaming KMSAN I think we'd better drop USB checks to reduce the patch set. It has already become enormous and hard to review or manage. -- Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Straße, 33 80636 München Geschäftsführer: Paul Manicle, Halimah DeLaine Prado Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg
