All of lore.kernel.org
 help / color / mirror / Atom feed
* [net:master 9/12] net/ipv6/ip6_offload.c:120:7-21: WARNING: Unsigned expression compared with zero: unfrag_ip6hlen < 0 (fwd)
@ 2017-05-18  2:01 Julia Lawall
  2017-05-18  2:58 ` David Miller
  0 siblings, 1 reply; 3+ messages in thread
From: Julia Lawall @ 2017-05-18  2:01 UTC (permalink / raw)
  To: Craig Gallek; +Cc: netdev, kbuild-all

Hello,

It may be worth checking on these.  The code context is shown in the first
case (line 120).  For the others, at least it gives the line numbers.

julia

---------- Forwarded message ----------
Date: Thu, 18 May 2017 09:07:31 +0800
From: kbuild test robot <fengguang.wu@intel.com>
To: kbuild@01.org
Cc: Julia Lawall <julia.lawall@lip6.fr>
Subject: [net:master 9/12] net/ipv6/ip6_offload.c:120:7-21: WARNING: Unsigned
    expression compared with zero: unfrag_ip6hlen < 0

CC: kbuild-all@01.org
CC: netdev@vger.kernel.org
TO: Craig Gallek <kraig@google.com>

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git master
head:   579f1d926c66cc0bd3bd87b1fe2e091084b07430
commit: 2423496af35d94a87156b063ea5cedffc10a70a1 [9/12] ipv6: Prevent overrun when parsing v6 header options
:::::: branch date: 2 hours ago
:::::: commit date: 6 hours ago

>> net/ipv6/ip6_offload.c:120:7-21: WARNING: Unsigned expression compared with zero: unfrag_ip6hlen < 0
--
>> net/ipv6/ip6_output.c:601:5-9: WARNING: Unsigned expression compared with zero: hlen < 0
--
>> net/ipv6/udp_offload.c:94:6-20: WARNING: Unsigned expression compared with zero: unfrag_ip6hlen < 0

git remote add net https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git
git remote update net
git checkout 2423496af35d94a87156b063ea5cedffc10a70a1
vim +120 net/ipv6/ip6_offload.c

d1da932e Vlad Yasevich        2012-11-15  104
07b26c94 Steffen Klassert     2016-09-19  105  	gso_partial = !!(skb_shinfo(segs)->gso_type & SKB_GSO_PARTIAL);
07b26c94 Steffen Klassert     2016-09-19  106
d1da932e Vlad Yasevich        2012-11-15  107  	for (skb = segs; skb; skb = skb->next) {
d3e5e006 Eric Dumazet         2013-10-20  108  		ipv6h = (struct ipv6hdr *)(skb_mac_header(skb) + nhoff);
07b26c94 Steffen Klassert     2016-09-19  109  		if (gso_partial)
802ab55a Alexander Duyck      2016-04-10  110  			payload_len = skb_shinfo(skb)->gso_size +
802ab55a Alexander Duyck      2016-04-10  111  				      SKB_GSO_CB(skb)->data_offset +
802ab55a Alexander Duyck      2016-04-10  112  				      skb->head - (unsigned char *)(ipv6h + 1);
802ab55a Alexander Duyck      2016-04-10  113  		else
802ab55a Alexander Duyck      2016-04-10  114  			payload_len = skb->len - nhoff - sizeof(*ipv6h);
802ab55a Alexander Duyck      2016-04-10  115  		ipv6h->payload_len = htons(payload_len);
d3e5e006 Eric Dumazet         2013-10-20  116  		skb->network_header = (u8 *)ipv6h - skb->head;
d3e5e006 Eric Dumazet         2013-10-20  117
91a48a2e Hannes Frederic Sowa 2014-02-24  118  		if (udpfrag) {
d1da932e Vlad Yasevich        2012-11-15  119  			unfrag_ip6hlen = ip6_find_1stfragopt(skb, &prevhdr);
2423496a Craig Gallek         2017-05-16 @120  			if (unfrag_ip6hlen < 0)
2423496a Craig Gallek         2017-05-16  121  				return ERR_PTR(unfrag_ip6hlen);
d3e5e006 Eric Dumazet         2013-10-20  122  			fptr = (struct frag_hdr *)((u8 *)ipv6h + unfrag_ip6hlen);
d1da932e Vlad Yasevich        2012-11-15  123  			fptr->frag_off = htons(offset);
53b24b8f Ian Morris           2015-03-29  124  			if (skb->next)
d1da932e Vlad Yasevich        2012-11-15  125  				fptr->frag_off |= htons(IP6_MF);
d1da932e Vlad Yasevich        2012-11-15  126  			offset += (ntohs(ipv6h->payload_len) -
d1da932e Vlad Yasevich        2012-11-15  127  				   sizeof(struct frag_hdr));
d1da932e Vlad Yasevich        2012-11-15  128  		}

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [net:master 9/12] net/ipv6/ip6_offload.c:120:7-21: WARNING: Unsigned expression compared with zero: unfrag_ip6hlen < 0 (fwd)
  2017-05-18  2:01 [net:master 9/12] net/ipv6/ip6_offload.c:120:7-21: WARNING: Unsigned expression compared with zero: unfrag_ip6hlen < 0 (fwd) Julia Lawall
@ 2017-05-18  2:58 ` David Miller
  2017-05-18 12:57   ` Craig Gallek
  0 siblings, 1 reply; 3+ messages in thread
From: David Miller @ 2017-05-18  2:58 UTC (permalink / raw)
  To: julia.lawall; +Cc: kraig, netdev, kbuild-all

From: Julia Lawall <julia.lawall@lip6.fr>
Date: Thu, 18 May 2017 10:01:07 +0800 (SGT)

> It may be worth checking on these.  The code context is shown in the first
> case (line 120).  For the others, at least it gives the line numbers.
 ...
>>> net/ipv6/ip6_offload.c:120:7-21: WARNING: Unsigned expression compared with zero: unfrag_ip6hlen < 0
> --
>>> net/ipv6/ip6_output.c:601:5-9: WARNING: Unsigned expression compared with zero: hlen < 0
> --
>>> net/ipv6/udp_offload.c:94:6-20: WARNING: Unsigned expression compared with zero: unfrag_ip6hlen < 0

Sigh, this is worse than the bug the commit introducing these warnings
was trying to fix.

I've pushed the following:

====================
ipv6: Check ip6_find_1stfragopt() return value properly.

Do not use unsigned variables to see if it returns a negative
error or not.

Fixes: 2423496af35d ("ipv6: Prevent overrun when parsing v6 header options")
Reported-by: Julia Lawall <julia.lawall@lip6.fr>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/ipv6/ip6_offload.c | 9 ++++-----
 net/ipv6/ip6_output.c  | 7 +++----
 net/ipv6/udp_offload.c | 8 +++++---
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
index eab36ab..280268f 100644
--- a/net/ipv6/ip6_offload.c
+++ b/net/ipv6/ip6_offload.c
@@ -63,7 +63,6 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 	const struct net_offload *ops;
 	int proto;
 	struct frag_hdr *fptr;
-	unsigned int unfrag_ip6hlen;
 	unsigned int payload_len;
 	u8 *prevhdr;
 	int offset = 0;
@@ -116,10 +115,10 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 		skb->network_header = (u8 *)ipv6h - skb->head;
 
 		if (udpfrag) {
-			unfrag_ip6hlen = ip6_find_1stfragopt(skb, &prevhdr);
-			if (unfrag_ip6hlen < 0)
-				return ERR_PTR(unfrag_ip6hlen);
-			fptr = (struct frag_hdr *)((u8 *)ipv6h + unfrag_ip6hlen);
+			int err = ip6_find_1stfragopt(skb, &prevhdr);
+			if (err < 0)
+				return ERR_PTR(err);
+			fptr = (struct frag_hdr *)((u8 *)ipv6h + err);
 			fptr->frag_off = htons(offset);
 			if (skb->next)
 				fptr->frag_off |= htons(IP6_MF);
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 01deecd..d4a31be 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -597,11 +597,10 @@ int ip6_fragment(struct net *net, struct sock *sk, struct sk_buff *skb,
 	int ptr, offset = 0, err = 0;
 	u8 *prevhdr, nexthdr = 0;
 
-	hlen = ip6_find_1stfragopt(skb, &prevhdr);
-	if (hlen < 0) {
-		err = hlen;
+	err = ip6_find_1stfragopt(skb, &prevhdr);
+	if (err < 0)
 		goto fail;
-	}
+	hlen = err;
 	nexthdr = *prevhdr;
 
 	mtu = ip6_skb_dst_mtu(skb);
diff --git a/net/ipv6/udp_offload.c b/net/ipv6/udp_offload.c
index b348cff..a2267f8 100644
--- a/net/ipv6/udp_offload.c
+++ b/net/ipv6/udp_offload.c
@@ -29,6 +29,7 @@ static struct sk_buff *udp6_ufo_fragment(struct sk_buff *skb,
 	u8 frag_hdr_sz = sizeof(struct frag_hdr);
 	__wsum csum;
 	int tnl_hlen;
+	int err;
 
 	mss = skb_shinfo(skb)->gso_size;
 	if (unlikely(skb->len <= mss))
@@ -90,9 +91,10 @@ static struct sk_buff *udp6_ufo_fragment(struct sk_buff *skb,
 		/* Find the unfragmentable header and shift it left by frag_hdr_sz
 		 * bytes to insert fragment header.
 		 */
-		unfrag_ip6hlen = ip6_find_1stfragopt(skb, &prevhdr);
-		if (unfrag_ip6hlen < 0)
-			return ERR_PTR(unfrag_ip6hlen);
+		err = ip6_find_1stfragopt(skb, &prevhdr);
+		if (err < 0)
+			return ERR_PTR(err);
+		unfrag_ip6hlen = err;
 		nexthdr = *prevhdr;
 		*prevhdr = NEXTHDR_FRAGMENT;
 		unfrag_len = (skb_network_header(skb) - skb_mac_header(skb)) +
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [net:master 9/12] net/ipv6/ip6_offload.c:120:7-21: WARNING: Unsigned expression compared with zero: unfrag_ip6hlen < 0 (fwd)
  2017-05-18  2:58 ` David Miller
@ 2017-05-18 12:57   ` Craig Gallek
  0 siblings, 0 replies; 3+ messages in thread
From: Craig Gallek @ 2017-05-18 12:57 UTC (permalink / raw)
  To: David Miller; +Cc: julia.lawall, netdev, kbuild-all

On Wed, May 17, 2017 at 10:58 PM, David Miller <davem@davemloft.net> wrote:
> From: Julia Lawall <julia.lawall@lip6.fr>
> Date: Thu, 18 May 2017 10:01:07 +0800 (SGT)
>
>> It may be worth checking on these.  The code context is shown in the first
>> case (line 120).  For the others, at least it gives the line numbers.
>  ...
>>>> net/ipv6/ip6_offload.c:120:7-21: WARNING: Unsigned expression compared with zero: unfrag_ip6hlen < 0
>> --
>>>> net/ipv6/ip6_output.c:601:5-9: WARNING: Unsigned expression compared with zero: hlen < 0
>> --
>>>> net/ipv6/udp_offload.c:94:6-20: WARNING: Unsigned expression compared with zero: unfrag_ip6hlen < 0
>
> Sigh, this is worse than the bug the commit introducing these warnings
> was trying to fix.
Thanks for the fix, sorry I missed this.

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-05-18 12:57 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-18  2:01 [net:master 9/12] net/ipv6/ip6_offload.c:120:7-21: WARNING: Unsigned expression compared with zero: unfrag_ip6hlen < 0 (fwd) Julia Lawall
2017-05-18  2:58 ` David Miller
2017-05-18 12:57   ` Craig Gallek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.