netdev - [PATCH] udp: increment UDP_MIB

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 03 Oct 2012 09:28:48 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Julian Anastasov <ja@....bg>
Cc:	David Miller <davem@...emloft.net>, chris2553@...glemail.com,
	netdev@...r.kernel.org, gpiez@....de, Dave Jones <davej@...hat.com>
Subject: [PATCH] udp: increment UDP_MIB_NOPORTS in mcast receive

On Wed, 2012-10-03 at 02:24 +0300, Julian Anastasov wrote:
> 	Hello,
> 
> On Tue, 2 Oct 2012, Eric Dumazet wrote:
> 
> > > David, shouldnt we use a nh_rth_forward instead of a nh_rth_input in
> > > __mkroute_input() ?
> > > 
> > > (And change rt_cache_route() as well ?)
> > > 
> > > I am testing a patch right now.
> > 
> > Yeah, this patch seems to fix the bug for me.
> > 
> > [PATCH] ipv4: properly cache forward routes
> > 
> > commit d2d68ba9fe8 (ipv4: Cache input routes in fib_info nexthops.)
> > introduced a regression for forwarding.
> > 
> > This was hard to reproduce but the symptom was that packets were
> > delivered to local host instead of being forwarded.
> > 
> > Add a separate cache (nh_rth_forward) to solve the problem.
> 
> 	Can it be a problem related to fib_info reuse
> from different routes. For example, when local IP address
> is created for subnet we have:
> 
> broadcast 192.168.0.255 dev DEV  proto kernel  scope link  src 192.168.0.1
> 192.168.0.0/24 dev DEV  proto kernel  scope link  src 192.168.0.1
> local 192.168.0.1 dev DEV  proto kernel  scope host  src 192.168.0.1
> 
> 	The "dev DEV  proto kernel  scope link  src 192.168.0.1" is
> a reused fib_info structure where we put cached routes.
> The result can be same fib_info for 192.168.0.255 and
> 192.168.0.0/24. RTN_BROADCAST is cached only for input
> routes. Incoming broadcast to 192.168.0.255 can be cached
> and can cause problems for traffic forwarded to 192.168.0.0/24.
> So, this patch should solve the problem because it
> separates the broadcast from unicast traffic.
> 
> 	And the ip_route_input_slow caching will work for
> local and broadcast input routes (above routes 1 and 3) just
> because they differ in scope and use different fib_info.
> 
> 	Another possible failure is for output routes:
> 
> multicast 224.0.0.0/4 fib_info
> with unicast
> 192.168.0.0/24 fib_info
> 
> 	The multicast sets RTCF_MULTICAST | RTCF_LOCAL
> and can cause problems for generated unicast traffic on
> fib_info reuse. Depends on the scope, for multicast it is
> usually scope global, so may be it is difficult to happen
> in practice.
> 
> 	__mkroute_output works for local/unicast routes
> because they differ in scope.


Thanks Julian for these informations.

BTW, it seems we dont properly increase UDP MIB counters when a
multicast message is not delivered to at least one socket.

Lets fix this to ease future bug hunting.

I hate when "netstat -s" is useless and we have to use dropwatch to
figure out where we drop a frame.

[PATCH] udp: increment UDP_MIB_NOPORTS in multicast receive

We should increment UDP_MIB_NOPORTS in the case we found
no socket to deliver a copy of one incoming UDP message.

(RFC 4113 udpNoPorts)

Signed-off-by: Eric Dumazet <edumazet@...gle.com>
---
 net/ipv4/udp.c |    1 +
 net/ipv6/udp.c |    1 +
 2 files changed, 2 insertions(+)

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 79c8dbe..dfa73c5 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1591,6 +1591,7 @@ static int __udp4_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
 			sock_put(stack[i]);
 	} else {
 		kfree_skb(skb);
+		UDP_INC_STATS_BH(net, UDP_MIB_NOPORTS, udptable != &udp_table);
 	}
 	return 0;
 }
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index fc99972..0be9ac2 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -748,6 +748,7 @@ static int __udp6_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
 			sock_put(stack[i]);
 	} else {
 		kfree_skb(skb);
+		UDP6_INC_STATS_BH(net, UDP_MIB_NOPORTS, udptable != &udp_table);
 	}
 	return 0;
 }


--
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