* ipv6: fix headroom calculation in udp6_ufo_fragment
@ 2013-12-13 13:01 Mark Hambleton
2013-12-13 13:14 ` Mark Brown
2013-12-13 13:34 ` Hannes Frederic Sowa
0 siblings, 2 replies; 7+ messages in thread
From: Mark Hambleton @ 2013-12-13 13:01 UTC (permalink / raw)
To: 'Pravin B Shelar', 'Saran Neti',
'David S. Miller', 'Greg Kroah-Hartman',
'Hannes Frederic Sowa'
Cc: Mark Brown (broonie@linaro.org), 'netdev@vger.kernel.org',
'stable@vger.kernel.org'
Hi,
Following a recent commit to LTS and upstream I see the following warning emitted on an ARM32 build:
net/ipv6/udp_offload.c: In function 'udp6_ufo_fragment':
net/ipv6/udp_offload.c:88:22: error: comparison between pointer and integer [-Werror]
if (skb->mac_header < (tnl_hlen + frag_hdr_sz)) {
^
cc1: all warnings being treated as errors
make[2]: *** [net/ipv6/udp_offload.o] Error 1
make[1]: *** [net/ipv6] Error 2
The commit made the following changes:
- if (skb_headroom(skb) < (tnl_hlen + frag_hdr_sz)) {
+ if (skb->mac_header < (tnl_hlen + frag_hdr_sz)) {
Now, because of some defines in skbuff.h :
#if BITS_PER_LONG > 32
#define NET_SKBUFF_DATA_USES_OFFSET 1
#endif
#ifdef NET_SKBUFF_DATA_USES_OFFSET
typedef unsigned int sk_buff_data_t;
#else
typedef unsigned char *sk_buff_data_t;
#endif
On an ARM32 system sk_buff_data_t would be a pointer (long being 32 bits), meaning that skb->mac_header is a pointer and that you are comparing a pointer against a length, which doesn't look like it is correct to me?
I can see how this works when NET_SKBUFF_DATA_USES_OFFSET is defined, but is there a way to do this that works for both cases?
Hope this makes sense,
Mark
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: ipv6: fix headroom calculation in udp6_ufo_fragment
2013-12-13 13:01 ipv6: fix headroom calculation in udp6_ufo_fragment Mark Hambleton
@ 2013-12-13 13:14 ` Mark Brown
2013-12-13 13:34 ` Hannes Frederic Sowa
1 sibling, 0 replies; 7+ messages in thread
From: Mark Brown @ 2013-12-13 13:14 UTC (permalink / raw)
To: Mark Hambleton
Cc: 'Pravin B Shelar', 'Saran Neti',
'David S. Miller', 'Greg Kroah-Hartman',
'Hannes Frederic Sowa', 'netdev@vger.kernel.org',
'stable@vger.kernel.org'
[-- Attachment #1: Type: text/plain, Size: 282 bytes --]
On Fri, Dec 13, 2013 at 01:01:59PM +0000, Mark Hambleton wrote:
> Following a recent commit to LTS and upstream I see the following
> warning emitted on an ARM32 build:
That's 024abeeced731 in LTS and 0e033e04c2678 upstream (ipv6: fix
headroom calculation in udp6_ufo_fragment).
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: ipv6: fix headroom calculation in udp6_ufo_fragment
2013-12-13 13:01 ipv6: fix headroom calculation in udp6_ufo_fragment Mark Hambleton
2013-12-13 13:14 ` Mark Brown
@ 2013-12-13 13:34 ` Hannes Frederic Sowa
2013-12-13 13:42 ` Mark Brown
1 sibling, 1 reply; 7+ messages in thread
From: Hannes Frederic Sowa @ 2013-12-13 13:34 UTC (permalink / raw)
To: Mark Hambleton
Cc: 'Pravin B Shelar', 'Saran Neti',
'David S. Miller', 'Greg Kroah-Hartman',
Mark Brown (broonie@linaro.org), 'netdev@vger.kernel.org',
'stable@vger.kernel.org'
On Fri, Dec 13, 2013 at 01:01:59PM +0000, Mark Hambleton wrote:
> Hi,
>
> Following a recent commit to LTS and upstream I see the following warning emitted on an ARM32 build:
>
> net/ipv6/udp_offload.c: In function 'udp6_ufo_fragment':
> net/ipv6/udp_offload.c:88:22: error: comparison between pointer and integer [-Werror]
> if (skb->mac_header < (tnl_hlen + frag_hdr_sz)) {
> ^
> cc1: all warnings being treated as errors
> make[2]: *** [net/ipv6/udp_offload.o] Error 1
> make[1]: *** [net/ipv6] Error 2
>
> The commit made the following changes:
>
> - if (skb_headroom(skb) < (tnl_hlen + frag_hdr_sz)) {
> + if (skb->mac_header < (tnl_hlen + frag_hdr_sz)) {
>
> Now, because of some defines in skbuff.h :
>
> #if BITS_PER_LONG > 32
> #define NET_SKBUFF_DATA_USES_OFFSET 1
> #endif
>
> #ifdef NET_SKBUFF_DATA_USES_OFFSET
> typedef unsigned int sk_buff_data_t;
> #else
> typedef unsigned char *sk_buff_data_t;
> #endif
>
> On an ARM32 system sk_buff_data_t would be a pointer (long being 32 bits), meaning that skb->mac_header is a pointer and that you are comparing a pointer against a length, which doesn't look like it is correct to me?
>
> I can see how this works when NET_SKBUFF_DATA_USES_OFFSET is defined, but is there a way to do this that works for both cases?
Sorry, I don't know which version the LTS kernel is. Upstream does not use
sk_buff_data_t for mac_header any more.
Something like that might help:
(skb_mac_header(skb) < skb->head + tnl_hlen + frag_hdr_sz)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: ipv6: fix headroom calculation in udp6_ufo_fragment
2013-12-13 13:34 ` Hannes Frederic Sowa
@ 2013-12-13 13:42 ` Mark Brown
2013-12-13 14:12 ` Hannes Frederic Sowa
0 siblings, 1 reply; 7+ messages in thread
From: Mark Brown @ 2013-12-13 13:42 UTC (permalink / raw)
To: Mark Hambleton, 'Pravin B Shelar', 'Saran Neti',
'David S. Miller', 'Greg Kroah-Hartman',
'netdev@vger.kernel.org',
'stable@vger.kernel.org'
[-- Attachment #1: Type: text/plain, Size: 211 bytes --]
On Fri, Dec 13, 2013 at 02:34:53PM +0100, Hannes Frederic Sowa wrote:
> Sorry, I don't know which version the LTS kernel is. Upstream does not use
> sk_buff_data_t for mac_header any more.
This is with v3.10.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: ipv6: fix headroom calculation in udp6_ufo_fragment
2013-12-13 13:42 ` Mark Brown
@ 2013-12-13 14:12 ` Hannes Frederic Sowa
2013-12-13 15:00 ` Mark Hambleton
2013-12-17 20:01 ` David Miller
0 siblings, 2 replies; 7+ messages in thread
From: Hannes Frederic Sowa @ 2013-12-13 14:12 UTC (permalink / raw)
To: Mark Brown
Cc: Mark Hambleton, 'Pravin B Shelar', 'Saran Neti',
'David S. Miller', 'Greg Kroah-Hartman',
'netdev@vger.kernel.org',
'stable@vger.kernel.org'
On Fri, Dec 13, 2013 at 01:42:05PM +0000, Mark Brown wrote:
> On Fri, Dec 13, 2013 at 02:34:53PM +0100, Hannes Frederic Sowa wrote:
>
> > Sorry, I don't know which version the LTS kernel is. Upstream does not use
> > sk_buff_data_t for mac_header any more.
>
> This is with v3.10.
The change happend in 3.11. So I guess it was an oversight while backporting.
Following patch should help and should get backported to all stable kernels
<= 3.10.
[PATCH stable] ipv6: fix illegal mac_header comparison on 32bit
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
Patch intended for stable kernel <= 3.10.
net/ipv6/udp_offload.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/udp_offload.c b/net/ipv6/udp_offload.c
index e7359f9..e15a357 100644
--- a/net/ipv6/udp_offload.c
+++ b/net/ipv6/udp_offload.c
@@ -90,7 +90,7 @@ static struct sk_buff *udp6_ufo_fragment(struct sk_buff *skb,
/* Check if there is enough headroom to insert fragment header. */
tnl_hlen = skb_tnl_header_len(skb);
- if (skb->mac_header < (tnl_hlen + frag_hdr_sz)) {
+ if (skb_mac_header(skb) < skb->head + tnl_hlen + frag_hdr_sz) {
if (gso_pskb_expand_head(skb, tnl_hlen + frag_hdr_sz))
goto out;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: ipv6: fix headroom calculation in udp6_ufo_fragment
2013-12-13 14:12 ` Hannes Frederic Sowa
@ 2013-12-13 15:00 ` Mark Hambleton
2013-12-17 20:01 ` David Miller
1 sibling, 0 replies; 7+ messages in thread
From: Mark Hambleton @ 2013-12-13 15:00 UTC (permalink / raw)
To: Hannes Frederic Sowa, Mark Brown
Cc: 'Pravin B Shelar', 'Saran Neti',
'David S. Miller', 'Greg Kroah-Hartman',
'netdev@vger.kernel.org',
'stable@vger.kernel.org'
Thanks!
That fixes the warning and looks like it will work.
Mark
-----Original Message-----
From: Hannes Frederic Sowa [mailto:hannes@stressinduktion.org]
Sent: 13 December 2013 14:12
To: Mark Brown
Cc: Mark Hambleton; 'Pravin B Shelar'; 'Saran Neti'; 'David S. Miller'; 'Greg Kroah-Hartman'; 'netdev@vger.kernel.org'; 'stable@vger.kernel.org'
Subject: Re: ipv6: fix headroom calculation in udp6_ufo_fragment
On Fri, Dec 13, 2013 at 01:42:05PM +0000, Mark Brown wrote:
> On Fri, Dec 13, 2013 at 02:34:53PM +0100, Hannes Frederic Sowa wrote:
>
> > Sorry, I don't know which version the LTS kernel is. Upstream does not use
> > sk_buff_data_t for mac_header any more.
>
> This is with v3.10.
The change happend in 3.11. So I guess it was an oversight while backporting.
Following patch should help and should get backported to all stable kernels
<= 3.10.
[PATCH stable] ipv6: fix illegal mac_header comparison on 32bit
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
Patch intended for stable kernel <= 3.10.
net/ipv6/udp_offload.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/udp_offload.c b/net/ipv6/udp_offload.c
index e7359f9..e15a357 100644
--- a/net/ipv6/udp_offload.c
+++ b/net/ipv6/udp_offload.c
@@ -90,7 +90,7 @@ static struct sk_buff *udp6_ufo_fragment(struct sk_buff *skb,
/* Check if there is enough headroom to insert fragment header. */
tnl_hlen = skb_tnl_header_len(skb);
- if (skb->mac_header < (tnl_hlen + frag_hdr_sz)) {
+ if (skb_mac_header(skb) < skb->head + tnl_hlen + frag_hdr_sz) {
if (gso_pskb_expand_head(skb, tnl_hlen + frag_hdr_sz))
goto out;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: ipv6: fix headroom calculation in udp6_ufo_fragment
2013-12-13 14:12 ` Hannes Frederic Sowa
2013-12-13 15:00 ` Mark Hambleton
@ 2013-12-17 20:01 ` David Miller
1 sibling, 0 replies; 7+ messages in thread
From: David Miller @ 2013-12-17 20:01 UTC (permalink / raw)
To: hannes; +Cc: broonie, mark.hambleton, pshelar, Saran.Neti, gregkh, netdev,
stable
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Fri, 13 Dec 2013 15:12:27 +0100
> On Fri, Dec 13, 2013 at 01:42:05PM +0000, Mark Brown wrote:
>> On Fri, Dec 13, 2013 at 02:34:53PM +0100, Hannes Frederic Sowa wrote:
>>
>> > Sorry, I don't know which version the LTS kernel is. Upstream does not use
>> > sk_buff_data_t for mac_header any more.
>>
>> This is with v3.10.
>
> The change happend in 3.11. So I guess it was an oversight while backporting.
> Following patch should help and should get backported to all stable kernels
> <= 3.10.
>
> [PATCH stable] ipv6: fix illegal mac_header comparison on 32bit
>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Queued up for -stable, thanks Hannes.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-12-17 20:01 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-13 13:01 ipv6: fix headroom calculation in udp6_ufo_fragment Mark Hambleton
2013-12-13 13:14 ` Mark Brown
2013-12-13 13:34 ` Hannes Frederic Sowa
2013-12-13 13:42 ` Mark Brown
2013-12-13 14:12 ` Hannes Frederic Sowa
2013-12-13 15:00 ` Mark Hambleton
2013-12-17 20:01 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).