Re: [NET]: Update frag_list in pskb_trim

[David Miller <davem@davemloft.net>]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu, 13 Jul 2006 19:03:41 +1000

> [NET]: Update frag_list in pskb_trim
> 
> When pskb_trim has to defer to ___pksb_trim to trim the frag_list part of
> the packet, the frag_list is not updated to reflect the trimming.  This
> will usually work fine until you hit something that uses the packet length
> or tail from the frag_list.
> 
> Examples include esp_output and ip_fragment.
> 
> Another problem caused by this is that you can end up with a linear packet
> with a frag_list attached.
> 
> It is possible to get away with this if we audit everything to make sure
> that they always consult skb->len before going down onto frag_list.  In
> fact we can do the samething for the paged part as well to avoid copying
> the data area of the skb.  For now though, let's do the conservative fix
> and update frag_list.
> 
> Many thanks to Marco Berizzi for helping me to track down this bug.
> 
> This 4-year old bug took 3 months to track down.  Marco was very patient
> indeed :)
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

I happened to be looking at this function just the other day
and said to myself: "It only drops the fraglist if the whole
thing is trimmed, lazy and inefficient but seems to work"

Even the existing check for when to drop the whole fraglist
seems to be buggy, it only drops if the length drops below
headlen.  Paged and fraglist skbs are legal, so the proper
check would be to drop the entire fraglist when len drops
to headlen + the length of the paged portion (if any).

In any event leaving stale chunks there on the fraglist is
certainly a major bug, thanks for fixing this Herbert.

I bet a suitably placed assertion or two would have caught this
sooner.  One good spot would have been skb_cow_data(), where it has to
mince the fragments due to the presence of a frag_list.