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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 30 Jul 2014 15:45:30 +0200 From: Hannes Frederic Sowa <hannes@...essinduktion.org> To: Duan Jiong <duanj.fnst@...fujitsu.com> Cc: David Miller <davem@...emloft.net>, eric.dumazet@...il.com, netdev <netdev@...r.kernel.org> Subject: Re: [PATCH] net: fix the counter ICMP_MIB_INERRORS/ICMP6_MIB_INERRORS On Di, 2014-07-29 at 16:19 +0800, Duan Jiong wrote: > When dealing with ICMPv[46] Error Message, function icmp_socket_deliver() > and icmpv6_notify() do some valid checks on packet's length, but then some > protocols check packet's length redaudantly. So remove those duplicated > statements, and increase counter ICMP_MIB_INERRORS/ICMP6_MIB_INERRORS in > function icmp_socket_deliver() and icmpv6_notify() respectively. > > In addition, add missed counter in udp6/udplite6 when socket is NULL. I am ok with adding the error counters, but... > --- > net/ipv4/icmp.c | 4 +++- > net/ipv4/tcp_ipv4.c | 5 ----- > net/ipv6/icmp.c | 11 ++++++++--- > net/ipv6/udp.c | 8 ++++++-- > net/sctp/input.c | 5 ----- > 5 files changed, 17 insertions(+), 16 deletions(-) > > diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c > index 42b7bcf..092400e 100644 > --- a/net/ipv4/icmp.c > +++ b/net/ipv4/icmp.c > @@ -663,8 +663,10 @@ static void icmp_socket_deliver(struct sk_buff *skb, u32 info) > /* 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)) > + if (!pskb_may_pull(skb, iph->ihl * 4 + 8)) { > + ICMP_INC_STATS_BH(dev_net(skb->dev), ICMP_MIB_INERRORS); > return; > + } > > raw_icmp_error(skb, protocol, info); > > diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c > index 77cccda..715cf6b 100644 > --- a/net/ipv4/tcp_ipv4.c > +++ b/net/ipv4/tcp_ipv4.c > @@ -342,11 +342,6 @@ void tcp_v4_err(struct sk_buff *icmp_skb, u32 info) > int err; > struct net *net = dev_net(icmp_skb->dev); > > - if (icmp_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(icmp_skb)); > if (!sk) { > diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c > index f6c84a6..8ca3cc0 100644 > --- a/net/ipv6/icmp.c > +++ b/net/ipv6/icmp.c > @@ -626,9 +626,10 @@ void icmpv6_notify(struct sk_buff *skb, u8 type, u8 code, __be32 info) > int inner_offset; > __be16 frag_off; > u8 nexthdr; > + struct net *net = dev_net(skb->dev); > > if (!pskb_may_pull(skb, sizeof(struct ipv6hdr))) > - return; > + goto out; > > nexthdr = ((struct ipv6hdr *)skb->data)->nexthdr; > if (ipv6_ext_hdr(nexthdr)) { > @@ -636,14 +637,14 @@ void icmpv6_notify(struct sk_buff *skb, u8 type, u8 code, __be32 info) > inner_offset = ipv6_skip_exthdr(skb, sizeof(struct ipv6hdr), > &nexthdr, &frag_off); > if (inner_offset<0) > - return; > + goto out; > } else { > inner_offset = sizeof(struct ipv6hdr); > } > > /* Checkin header including 8 bytes of inner protocol header. */ > if (!pskb_may_pull(skb, inner_offset+8)) > - return; > + goto out; > > /* BUGGG_FUTURE: we should try to parse exthdrs in this packet. > Without this we will not able f.e. to make source routed > @@ -659,6 +660,10 @@ void icmpv6_notify(struct sk_buff *skb, u8 type, u8 code, __be32 info) > rcu_read_unlock(); > > raw6_icmp_error(skb, nexthdr, type, code, inner_offset, info); > + return; > + > +out: > + ICMP6_INC_STATS_BH(net, __in6_dev_get(skb->dev), ICMP6_MIB_INERRORS); ... please don't use __in6_dev_get in BH without rcu, but inet6_iif. You risk a lockdep error otherwise. You don't have rtnl locked and no RCU read lock. > } > > /* > diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c > index 7092ff7..f13d297 100644 > --- a/net/ipv6/udp.c > +++ b/net/ipv6/udp.c > @@ -534,11 +534,15 @@ void __udp6_lib_err(struct sk_buff *skb, struct inet6_skb_parm *opt, > struct udphdr *uh = (struct udphdr*)(skb->data+offset); > struct sock *sk; > int err; > + struct net *net = dev_net(skb->dev); > > - sk = __udp6_lib_lookup(dev_net(skb->dev), daddr, uh->dest, > + sk = __udp6_lib_lookup(net, daddr, uh->dest, > saddr, uh->source, inet6_iif(skb), udptable); > - if (sk == NULL) > + if (sk == NULL) { > + ICMP6_INC_STATS_BH(net, __in6_dev_get(skb->dev), > + ICMP6_MIB_INERRORS); It is not really necessary here, as you like. Bye, Hannes -- 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