[Xen-devel] RE: [PATCH ixgbe] Don't depend of skb->data for VMDq

["Williams, Mitch A" <mitch.a.williams@xxxxxxxxx>]

>> Occasionally, vmq_alloc_skb() will give us an skb with a nonzero but
>> bogus data pointer.  Dereferencing this pointer will then cause a
>> panic.
>That's disturbing.  The SKB needs to have a data area (because
>otherwise there's nowhere to put the skb_shinfo, and hence no frags
>list), so the data pointer really ought to be valid (i.e. NULL is also
>bad here).
>

OK, I need to eat some crow here.  The patch is correct, but my description is 
completely wrong, and is based on false assumptions.  I completely forgot that 
skb_shinfo hangs off the data area, and assumed that "no packet data in 
skb->data" was equivalent to "skb->data is NULL".  Completely, totally, wrong.

As you noted, skb->data has got to be valid for the skb to be useful at all.  
The use of "if (skb->data)" is completely incorrect in any context.  I should 
know this -- I've stared at skbuff.h long enough over the years.

In my defense, the patch went through two internal review cycles and nobody 
else noticed it either.  So I feel a little better.  It's a use case that just 
doesn't come up that often.

Please keep the patch, but ignore (or change) the description.  Sorry about the 
confusion.

-Mitch
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel