[PATCH] ipv4: udp: fix short packet and bad checksum logging

[PATCH] ipv4: udp: fix short packet and bad checksum logging

[Bjørn Mork]

commit 2783ef23 moved the initialisation of saddr and daddr after
pskb_may_pull() to avoid a potential data corruption.  Unfortunately
also placing it after the short packet and bad checksum error paths,
where these variables are used for logging.  The result is bogus
output like

[92238.389505] UDP: short packet: From 2.0.0.0:65535 23715/178 to 0.0.0.0:65535

Moving the saddr and daddr initialisation above the error paths, while still
keeping it after the pskb_may_pull() to keep the fix from commit 2783ef23.

Signed-off-by: Bjørn Mork <bjorn@mork.no>
Cc: stable@kernel.org
---
 net/ipv4/udp.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 8fef859..c36522a 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1527,6 +1527,9 @@ int __udp4_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
 
 	uh   = udp_hdr(skb);
 	ulen = ntohs(uh->len);
+	saddr = ip_hdr(skb)->saddr;
+	daddr = ip_hdr(skb)->daddr;
+
 	if (ulen > skb->len)
 		goto short_packet;
 
@@ -1540,9 +1543,6 @@ int __udp4_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
 	if (udp4_csum_init(skb, uh, proto))
 		goto csum_error;
 
-	saddr = ip_hdr(skb)->saddr;
-	daddr = ip_hdr(skb)->daddr;
-
 	if (rt->rt_flags & (RTCF_BROADCAST|RTCF_MULTICAST))
 		return __udp4_lib_mcast_deliver(net, skb, uh,
 				saddr, daddr, udptable);
-- 
1.5.6.5

[PATCH] ipv6: udp: make short packet logging consistent with ipv4

[Bjørn Mork]

Adding addresses and ports to the short packet log message,
like ipv4/udp.c does it, makes these messages a lot more useful:

[  822.182450] UDPv6: short packet: From [2001:db8:ffb4:3::1]:47839 23715/178 to [2001:db8:ffb4:3:5054:ff:feff:200]:1234

This requires us to drop logging in case pskb_may_pull() fails,
which also is consistent with ipv4/udp.c

Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 net/ipv6/udp.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 9082485..d799244 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -692,7 +692,7 @@ int __udp6_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
 	u32 ulen = 0;
 
 	if (!pskb_may_pull(skb, sizeof(struct udphdr)))
-		goto short_packet;
+		goto discard;
 
 	saddr = &ipv6_hdr(skb)->saddr;
 	daddr = &ipv6_hdr(skb)->daddr;
@@ -770,9 +770,14 @@ int __udp6_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
 	return 0;
 
 short_packet:
-	LIMIT_NETDEBUG(KERN_DEBUG "UDP%sv6: short packet: %d/%u\n",
+	LIMIT_NETDEBUG(KERN_DEBUG "UDP%sv6: short packet: From [%pI6c]:%u %d/%d to [%pI6c]:%u\n",
 		       proto == IPPROTO_UDPLITE ? "-Lite" : "",
-		       ulen, skb->len);
+		       saddr,
+		       ntohs(uh->source),
+		       ulen,
+		       skb->len,
+		       daddr,
+		       ntohs(uh->dest));
 
 discard:
 	UDP6_INC_STATS_BH(net, UDP_MIB_INERRORS, proto == IPPROTO_UDPLITE);
-- 
1.5.6.5

Re: [PATCH] ipv4: udp: fix short packet and bad checksum logging

[Eric Dumazet]

Le jeudi 06 mai 2010 à 15:44 +0200, Bjørn Mork a écrit :
> commit 2783ef23 moved the initialisation of saddr and daddr after
> pskb_may_pull() to avoid a potential data corruption.  Unfortunately
> also placing it after the short packet and bad checksum error paths,
> where these variables are used for logging.  The result is bogus
> output like
> 
> [92238.389505] UDP: short packet: From 2.0.0.0:65535 23715/178 to 0.0.0.0:65535
> 
> Moving the saddr and daddr initialisation above the error paths, while still
> keeping it after the pskb_may_pull() to keep the fix from commit 2783ef23.
> 
> Signed-off-by: Bjørn Mork <bjorn@mork.no>
> Cc: stable@kernel.org
> ---
>  net/ipv4/udp.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)

Well done :)

Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

To be backported to 2.6.29 and up kernels ;)

Re: [PATCH] ipv6: udp: make short packet logging consistent with ipv4

[Eric Dumazet]

Le jeudi 06 mai 2010 à 15:44 +0200, Bjørn Mork a écrit :
> Adding addresses and ports to the short packet log message,
> like ipv4/udp.c does it, makes these messages a lot more useful:
> 
> [  822.182450] UDPv6: short packet: From [2001:db8:ffb4:3::1]:47839 23715/178 to [2001:db8:ffb4:3:5054:ff:feff:200]:1234
> 
> This requires us to drop logging in case pskb_may_pull() fails,
> which also is consistent with ipv4/udp.c
> 
> Signed-off-by: Bjørn Mork <bjorn@mork.no>
> ---
>  net/ipv6/udp.c |   11 ++++++++---
>  1 files changed, 8 insertions(+), 3 deletions(-)

Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

Re: [PATCH] ipv4: udp: fix short packet and bad checksum logging

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 06 May 2010 16:48:00 +0200

> Le jeudi 06 mai 2010 à 15:44 +0200, Bjørn Mork a écrit :
>> commit 2783ef23 moved the initialisation of saddr and daddr after
>> pskb_may_pull() to avoid a potential data corruption.  Unfortunately
>> also placing it after the short packet and bad checksum error paths,
>> where these variables are used for logging.  The result is bogus
>> output like
>> 
>> [92238.389505] UDP: short packet: From 2.0.0.0:65535 23715/178 to 0.0.0.0:65535
>> 
>> Moving the saddr and daddr initialisation above the error paths, while still
>> keeping it after the pskb_may_pull() to keep the fix from commit 2783ef23.
>> 
>> Signed-off-by: Bjørn Mork <bjorn@mork.no>
>> Cc: stable@kernel.org
>> ---
>>  net/ipv4/udp.c |    6 +++---
>>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> Well done :)
> 
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
> 
> To be backported to 2.6.29 and up kernels ;)

Applied to net-2.6 and queued up for -stable, thanks!

Re: [PATCH] ipv6: udp: make short packet logging consistent with ipv4

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 06 May 2010 16:49:26 +0200

> Le jeudi 06 mai 2010 à 15:44 +0200, Bjørn Mork a écrit :
>> Adding addresses and ports to the short packet log message,
>> like ipv4/udp.c does it, makes these messages a lot more useful:
>> 
>> [  822.182450] UDPv6: short packet: From [2001:db8:ffb4:3::1]:47839 23715/178 to [2001:db8:ffb4:3:5054:ff:feff:200]:1234
>> 
>> This requires us to drop logging in case pskb_may_pull() fails,
>> which also is consistent with ipv4/udp.c
>> 
>> Signed-off-by: Bjørn Mork <bjorn@mork.no>
>> ---
>>  net/ipv6/udp.c |   11 ++++++++---
>>  1 files changed, 8 insertions(+), 3 deletions(-)
> 
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied to net-next-2.6, thanks.