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