* [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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox