On Tue, 27 Oct 2009 15:29:15 -0400 (EDT), Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> This patch (as1299) fixes a bug in an error-handling path of usbmon's
> binary interface. The storage area for URB data is divided into
> fixed-size blocks. If an URB's data can't be copied, the area
> reserved for it should be decreased to the size of the truncated
> information (rounded up to a block boundary). Rounding up the amount
> to be removed and subtracting it from the reserved size is definitely
> the wrong thing to do.
Right, I see. Still, the solution seems a bit heavy handed.
I find it easier to reason about mon_buff_area_shrink if it remains
a compliment to mon_buff_area_free.
> Also, when the data for an isochronous URB can't be copied, we can
> still copy the isoc packet descriptors. In fact the current code does
> copy the descriptors, but then sets the capture length to 0 so they
> become inaccessible. The capture length should be reduced to the
> length of the descriptors, not set to 0.
That part is fine, I see that we should do it.
How about the attached? It should be doing exactly what your patch
did, only reformatted a bit, and keeps mon_buff_area_shrink focused.
-- Pete
diff --git a/drivers/usb/mon/mon_bin.c b/drivers/usb/mon/mon_bin.c
index 9ed3e74..6469a8d 100644
--- a/drivers/usb/mon/mon_bin.c
+++ b/drivers/usb/mon/mon_bin.c
@@ -348,12 +348,12 @@ static unsigned int mon_buff_area_alloc_contiguous(struct mon_reader_bin *rp,
/*
* Return a few (kilo-)bytes to the head of the buffer.
- * This is used if a DMA fetch fails.
+ * This is used if a data fetch fails.
*/
static void mon_buff_area_shrink(struct mon_reader_bin *rp, unsigned int size)
{
- size = (size + PKT_ALIGN-1) & ~(PKT_ALIGN-1);
+ /* size &= ~(PKT_ALIGN-1); -- we're called with aligned size */
rp->b_cnt -= size;
if (rp->b_in < size)
rp->b_in += rp->b_size;
@@ -433,6 +433,7 @@ static void mon_bin_event(struct mon_reader_bin *rp, struct urb *urb,
unsigned int urb_length;
unsigned int offset;
unsigned int length;
+ unsigned int delta;
unsigned int ndesc, lendesc;
unsigned char dir;
struct mon_bin_hdr *ep;
@@ -537,8 +538,10 @@ static void mon_bin_event(struct mon_reader_bin *rp, struct urb *urb,
if (length != 0) {
ep->flag_data = mon_bin_get_data(rp, offset, urb, length);
if (ep->flag_data != 0) { /* Yes, it's 0x00, not '0' */
- ep->len_cap = 0;
- mon_buff_area_shrink(rp, length);
+ delta = (ep->len_cap + PKT_ALIGN-1) & ~(PKT_ALIGN-1);
+ delta -= (lendesc + PKT_ALIGN-1) & ~(PKT_ALIGN-1);
+ ep->len_cap = lendesc;
+ mon_buff_area_shrink(rp, delta);
}
} else {
ep->flag_data = data_tag;
--
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
