Pete: Has this been brought up before? Something similar went by a few weeks ago but I don't think it was exactly the same. One aspect of this worries me: By copying the entire buffer, we risk leaking kernel data. What do you think? Should mon_submit() zero out the buffer for isochronous-IN transfers? Alan Stern --- usbmon doesn't copy the right number of bytes to its output buffer for isochronous-IN transfers. It should copy the entire transfer buffer instead of just the first urb->actual_length bytes, because isochronous data is stored in a discontiguous fashion, especially when some of the data packets were not received. This patch (as1439) fixes the problem by setting the copy length to urb->transfer_buffer_length for isochronous completions. (If the URB was an OUT transfer instead of IN, existing code farther down will set the length to 0.) Index: usb-2.6/Documentation/usb/usbmon.txt =================================================================== --- usb-2.6.orig/Documentation/usb/usbmon.txt +++ usb-2.6/Documentation/usb/usbmon.txt @@ -162,7 +162,11 @@ Here is the list of words, from left to not machine words, but really just a byte stream split into words to make it easier to read. Thus, the last word may contain from one to four bytes. The length of collected data is limited and can be less than the data length - report in Data Length word. + reported in the Data Length word. In the case of an Isochronous-IN + completion where some of the data packets were not received, the length of + the collected data can be greater than the Data Length value (because Data + Length counts only the bytes that were received whereas the Data words + contain the entire transfer buffer). Here is an example of code to read the data stream in a well known programming language: Index: usb-2.6/drivers/usb/mon/mon_bin.c =================================================================== --- usb-2.6.orig/drivers/usb/mon/mon_bin.c +++ usb-2.6/drivers/usb/mon/mon_bin.c @@ -494,6 +494,8 @@ static void mon_bin_event(struct mon_rea urb_length = (ev_type == 'S') ? urb->transfer_buffer_length : urb->actual_length; length = urb_length; + if (ndesc > 0 && ev_type == 'C') + length = urb->transfer_buffer_length; if (length >= rp->b_size/5) length = rp->b_size/5; Index: usb-2.6/drivers/usb/mon/mon_text.c =================================================================== --- usb-2.6.orig/drivers/usb/mon/mon_text.c +++ usb-2.6/drivers/usb/mon/mon_text.c @@ -236,6 +236,8 @@ static void mon_text_event(struct mon_re fp++; dp++; } + if (ev_type == 'C') + ep->length = urb->transfer_buffer_length; } ep->setup_flag = mon_text_get_setup(ep, urb, ev_type, rp->r.m_bus); -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
