[PATCH ipsec] net: xfrm: pass constant family to nf_hook function

[PATCH ipsec] net: xfrm: pass constant family to nf_hook function

[Florian Westphal]

Unfortunately some versions of gcc emit following warning:
  linux/compiler.h:252:20: warning: array subscript is above array bounds [-Warray-bounds]
  hook_head = rcu_dereference(net->nf.hooks_arp[hook]);
                              ^~~~~~~~~~~~~~~~~~~~~
xfrm_output_resume passes non-const 'pf' argument so compiler can't know
that the affected statement above will never be executed (we only
pass either NFPROTO_IPV4 or NFPROTO_IPV6), this change makes this
explicit.

Another solution would be to increase hooks_arp[] size, but that
increases struct net size needlesly.

Reported-by: David Ahern <dsahern@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 David, i hope this will silence the warning, would be nice
 if you could test it.

 I don't really like this patch, but I see no better solution
 expect needless increase of hooks_arp[].

 Any other idea?

 net/xfrm/xfrm_output.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index 45ba07ab3e4f..199c0e782ac7 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -152,11 +152,24 @@ int xfrm_output_resume(struct sk_buff *skb, int err)
 		if (!skb_dst(skb)->xfrm)
 			return dst_output(net, skb->sk, skb);
 
-		err = nf_hook(skb_dst(skb)->ops->family,
-			      NF_INET_POST_ROUTING, net, skb->sk, skb,
-			      NULL, skb_dst(skb)->dev, xfrm_output2);
-		if (unlikely(err != 1))
-			goto out;
+		switch (skb_dst(skb)->ops->family) {
+		case AF_INET:
+			err = nf_hook(NFPROTO_IPV4,
+				      NF_INET_POST_ROUTING, net, skb->sk, skb,
+				      NULL, skb_dst(skb)->dev, xfrm_output2);
+			if (unlikely(err != 1))
+				goto out;
+			break;
+		case AF_INET6:
+			err = nf_hook(NFPROTO_IPV6,
+				      NF_INET_POST_ROUTING, net, skb->sk, skb,
+				      NULL, skb_dst(skb)->dev, xfrm_output2);
+			if (unlikely(err != 1))
+				goto out;
+			break;
+		default:
+			break;
+		}
 	}
 
 	if (err == -EINPROGRESS)
-- 
2.16.4

Re: [PATCH ipsec] net: xfrm: pass constant family to nf_hook function

[David Ahern]

On 9/21/18 3:35 AM, Florian Westphal wrote:
> Unfortunately some versions of gcc emit following warning:
>   linux/compiler.h:252:20: warning: array subscript is above array bounds [-Warray-bounds]
>   hook_head = rcu_dereference(net->nf.hooks_arp[hook]);
>                               ^~~~~~~~~~~~~~~~~~~~~
> xfrm_output_resume passes non-const 'pf' argument so compiler can't know
> that the affected statement above will never be executed (we only
> pass either NFPROTO_IPV4 or NFPROTO_IPV6), this change makes this
> explicit.
> 
> Another solution would be to increase hooks_arp[] size, but that
> increases struct net size needlesly.
> 
> Reported-by: David Ahern <dsahern@gmail.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  David, i hope this will silence the warning, would be nice
>  if you could test it.

I still the warning.

> 
>  I don't really like this patch, but I see no better solution
>  expect needless increase of hooks_arp[].
> 
>  Any other idea?
> 

I don't have any time in the next week to look into it.

Sounds like you are not able to reproduce the output. It just debian
stretch and gcc version 6.3.0 20170516 (Debian 6.3.0-18+deb9u1).

Re: [PATCH ipsec] net: xfrm: pass constant family to nf_hook function

[Florian Westphal]

David Ahern <dsahern@gmail.com> wrote:
> >  David, i hope this will silence the warning, would be nice
> >  if you could test it.
> 
> I still the warning.

Wait.  Do you see this warning everywhere or just in xfrm?

Re: [PATCH ipsec] net: xfrm: pass constant family to nf_hook function

[David Ahern]

On 9/21/18 8:55 AM, Florian Westphal wrote:
> David Ahern <dsahern@gmail.com> wrote:
>>>  David, i hope this will silence the warning, would be nice
>>>  if you could test it.
>>
>> I still the warning.
> 
> Wait.  Do you see this warning everywhere or just in xfrm?
> 

just the one file.

Re: [PATCH ipsec] net: xfrm: pass constant family to nf_hook function

[Florian Westphal]

David Ahern <dsahern@gmail.com> wrote:
> On 9/21/18 8:55 AM, Florian Westphal wrote:
> > David Ahern <dsahern@gmail.com> wrote:
> >>>  David, i hope this will silence the warning, would be nice
> >>>  if you could test it.
> >>
> >> I still the warning.
> > 
> > Wait.  Do you see this warning everywhere or just in xfrm?
> > 
> 
> just the one file.

Sigh, ok, i will set up a vm and deal with this somehow.

Steffen, please toss this patch, sorry for the noise.