Messages in this thread | | | Date | Fri, 09 Jul 2010 17:00:18 -0700 (PDT) | Subject | Re: net/sched/act_nat.c BUG | From | David Miller <> |
| |
From: Herbert Xu <herbert@gondor.apana.org.au> Date: Sat, 10 Jul 2010 07:23:59 +0800
> On Fri, Jul 09, 2010 at 05:13:40PM +0200, Eric Dumazet wrote: >> Le vendredi 09 juillet 2010 à 16:37 +0200, Rodrigo Partearroyo González >> a écrit : >> > Hi all, >> > >> > I have been testing Stateless NAT and found that ICMP packets with length less >> > than 20 bytes were not correctly NAT'ed. I have found a BUG that makes taking >> > into account IP header length twice, so ICMP packets smaller than 20 bytes >> > were being dropped. >> > >> >> CC netdev >> >> > The proposed fix is: >> > >> > Index: net/sched/act_nat.c >> > =================================================================== >> > --- net/sched/act_nat.c >> > +++ net/sched/act_nat.c >> > @@ -202,7 +202,7 @@ >> > { >> > struct icmphdr *icmph; >> > >> > - if (!pskb_may_pull(skb, ihl + sizeof(*icmph) + sizeof(*iph))) >> > + if (!pskb_may_pull(skb, ihl + sizeof(*icmph))) >> > goto drop; >> > >> > icmph = (void *)(skb_network_header(skb) + ihl); >> > >> > Please, consider applying it. >> >> Nice catch, but take a look at next lines too, >> when call to skb_clone_writable() is done, since same error is present. >> >> skb_clone_writable(skb, >> ihl + sizeof(*icmph) + sizeof(*iph)) >> >> Please submit a formal patch, with your "Signed-off-by: ...", as >> documented in Documentation/SubmittingPatches > > No we do need the second IP header, think about it... > > However, we should only drop it only if it's long enough and > pskb_may_pull fails.
Ok, I've reverted until we come up with the proper fix, thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |