Re: [PATCH] cls_u32: use skb_copy_bits() to dereference data safely

[Changli Gao <xiaosuo@gmail.com>]

On Wed, Jun 2, 2010 at 8:45 PM, David Miller <davem@davemloft.net> wrote:
> From: jamal <hadi@cyberus.ca>
> Date: Wed, 02 Jun 2010 08:25:38 -0400
>
>> --- a/net/sched/cls_u32.c
>> +++ b/net/sched/cls_u32.c
>> @@ -135,6 +135,9 @@ next_knode:
>>
>>  for (i = n->sel.nkeys; i>0; i--, key++) {
>>
>> +        int toff = key->off+(off2&key->offmask)- 4;
>> +        if (unlikely(toff > skb->len))
>> +              /* bailout here - needs some thought */
>>          if ((*(__be32*)(ptr+key->off+(off2&key->offmask))^key->v
>
> I don't think it's that simple.
>
> You can't dereference from the skb->data linear area if your offset is
> beyond "skb->len - skb->data_len" (aka. skb_headlen()) since that's
> where the paged or fragmented portion starts.
>
> We really need to use skb_copy_bits() if we want to allow
> any offset into the SKB, and because of all the ways
> packets can be transformed and constructed we absolutely
> have to.
>

Maybe skb_header_pointer() is lighter.


-- 
Regards,
Changli Gao(xiaosuo@gmail.com)