Message ID | 20130124230850.GI8541@breakpoint.cc |
---|---|
State | Superseded |
Headers | show |
On Friday 2013-01-25 00:08, Florian Westphal wrote: >@@ -35,10 +35,18 @@ tcpoptstrip_mangle_packet(struct sk_buff *skb, > { > unsigned int optl, i, j; > struct tcphdr *tcph; >+ struct tcphdr _tcph; > u_int16_t n, o; > u_int8_t *opt; > >- if (!skb_make_writable(skb, skb->len)) >+ if (skb->len < minlen) >+ return XT_CONTINUE; >+ >+ tcph = skb_header_pointer(skb, tcphoff, sizeof(_tcph), &_tcph); >+ if (!tcph) >+ return XT_CONTINUE; /* no options -> nothing to do */ To the best of my analysis, the "no options" comment is incorrect here, because you are not even looking at the options so far, but only tcph. The prose should probably be something like: if (iph->frag_off & htons(IP_OFFSET)) != 0) /* not the first fragment - lost case */ return XT_CONTINUE; if (iph->len < iph->ihl * 4 + sizeof(_tcph)) /* fragment boundary within tcphdr */ return XT_CONTINUE; tcph = skb_header_pointer(skb, tcphoff, sizeof(_tcph), &_tcph); if (tcph == NULL) /* packet way too short for some reason (should not occur since we tested for fragment boundary) */ return NF_DROP; if (tcph->doff * 4 - sizeof(_tcph) == 0) /* no options */ return XT_CONTINUE; if (iph->len < iph->ihl * 4 + sizeof(_tcph) + tcphoff)) /* fragment boundary within tcpoptions */ return XT_CONTINUE; skb_make_writable... -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Jan Engelhardt <jengelh@inai.de> wrote: > On Friday 2013-01-25 00:08, Florian Westphal wrote: > >@@ -35,10 +35,18 @@ tcpoptstrip_mangle_packet(struct sk_buff *skb, > > { > > unsigned int optl, i, j; > > struct tcphdr *tcph; > >+ struct tcphdr _tcph; > > u_int16_t n, o; > > u_int8_t *opt; > > > >- if (!skb_make_writable(skb, skb->len)) > >+ if (skb->len < minlen) > >+ return XT_CONTINUE; > >+ > >+ tcph = skb_header_pointer(skb, tcphoff, sizeof(_tcph), &_tcph); > >+ if (!tcph) > >+ return XT_CONTINUE; /* no options -> nothing to do */ > > To the best of my analysis, the "no options" comment is incorrect here, > because you are not even looking at the options so far, but only tcph. Yup. > The prose should probably be something like: > if (iph->frag_off & htons(IP_OFFSET)) != 0) > /* not the first fragment - lost case */ > return XT_CONTINUE; [..] Can to submit a patch? -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ipv4/netfilter/ipt_ECN.c b/net/ipv4/netfilter/ipt_ECN.c index 4bf3dc4..a1f8a59 100644 --- a/net/ipv4/netfilter/ipt_ECN.c +++ b/net/ipv4/netfilter/ipt_ECN.c @@ -86,7 +86,7 @@ ecn_tg(struct sk_buff *skb, const struct xt_action_param *par) return NF_DROP; if (einfo->operation & (IPT_ECN_OP_SET_ECE | IPT_ECN_OP_SET_CWR) && - ip_hdr(skb)->protocol == IPPROTO_TCP) + (ip_hdr(skb)->frag_off & htons(IP_OFFSET)) == 0) if (!set_ect_tcp(skb, einfo)) return NF_DROP; diff --git a/net/netfilter/xt_TCPOPTSTRIP.c b/net/netfilter/xt_TCPOPTSTRIP.c index 25fd1c4..ebb9451 100644 --- a/net/netfilter/xt_TCPOPTSTRIP.c +++ b/net/netfilter/xt_TCPOPTSTRIP.c @@ -35,10 +35,18 @@ tcpoptstrip_mangle_packet(struct sk_buff *skb, { unsigned int optl, i, j; struct tcphdr *tcph; + struct tcphdr _tcph; u_int16_t n, o; u_int8_t *opt; - if (!skb_make_writable(skb, skb->len)) + if (skb->len < minlen) + return XT_CONTINUE; + + tcph = skb_header_pointer(skb, tcphoff, sizeof(_tcph), &_tcph); + if (!tcph) + return XT_CONTINUE; /* no options -> nothing to do */ + + if (!skb_make_writable(skb, tcphoff + (tcph->doff * 4))) return NF_DROP; tcph = (struct tcphdr *)(skb_network_header(skb) + tcphoff); @@ -76,6 +84,9 @@ tcpoptstrip_mangle_packet(struct sk_buff *skb, static unsigned int tcpoptstrip_tg4(struct sk_buff *skb, const struct xt_action_param *par) { + if (ip_hdr(skb)->frag_off & htons(IP_OFFSET)) + return XT_CONTINUE; + return tcpoptstrip_mangle_packet(skb, par->targinfo, ip_hdrlen(skb), sizeof(struct iphdr) + sizeof(struct tcphdr)); }