lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 25 Jul 2008 22:15:33 -0400 From: Vlad Yasevich <vladislav.yasevich@...com> To: Gerrit Renker <gerrit@....abdn.ac.uk>, netdev@...r.kernel.org Subject: Re: [RFC] sctp/tcp: Question -- ICMPv4 length check (not) redundant? Gerrit Renker wrote: > I wonder if the two pieces of code as marked by the patch below still > serve a purpose. > > From grepping through the net/ code it seems they are both redundant. > > This is on the basis that > > * the v6 counterparts of these handlers ({tcp,sctp}_v6_err) do not have > this payload-length check; > > * icmp_unreach() in net/ipv4/icmp.c already has this test: > > /* Checkin full IP header plus 8 bytes of protocol to > * avoid additional coding at protocol handlers. > */ > if (!pskb_may_pull(skb, iph->ihl * 4 + 8)) > goto out; > > * a similar test is implemented by icmpv6_notify() in net/ipv6/icmp.c; > > * this type of test seems to be absent in all of the following handlers: > - tcp_v6_err() - sctp_v6_err() > - __udp4_lib_err() - __udp6_lib_err() > - ah4_err() - ah6_err() > - esp4_err() - esp6_err() > - xfrm_tunnel_err() - xfrm6_tunnel_err() > - tunnel4_err() - tunnel6_err() - tunnel64_err() > - ipip_err() - ipip6_err() > - ip4ip6_err() - ip6ip6_err() > - ipcomp4_err() - ipcomp6_err() > > --------------------------------------------------------------------------- > The following two are the exceptions in question. > The question is whether there is a reason for the code marked below. > --------------------------------------------------------------------------- > > --- a/net/ipv4/tcp_ipv4.c > +++ b/net/ipv4/tcp_ipv4.c > @@ -344,11 +344,6 @@ void tcp_v4_err(struct sk_buff *skb, u32 info) > int err; > struct net *net = dev_net(skb->dev); > > - if (skb->len < (iph->ihl << 2) + 8) { > - ICMP_INC_STATS_BH(net, ICMP_MIB_INERRORS); > - return; > - } > - > sk = inet_lookup(net, &tcp_hashinfo, iph->daddr, th->dest, > iph->saddr, th->source, inet_iif(skb)); > if (!sk) { > --- a/net/sctp/input.c > +++ b/net/sctp/input.c > @@ -534,11 +534,6 @@ void sctp_v4_err(struct sk_buff *skb, __u32 info) > sk_buff_data_t saveip, savesctp; > int err; > > - if (skb->len < ihlen + 8) { > - ICMP_INC_STATS_BH(&init_net, ICMP_MIB_INERRORS); > - return; > - } > - > /* Fix up skb to look at the embedded net header. */ > saveip = skb->network_header; > savesctp = skb->transport_header; This one looks useless. Thanks for noticing. I'll remove it. -vlad > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@...r.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists