* [PATCH] [rfc, netfilter-next] netfilter: nf_tables: fib warnings
@ 2016-10-28 15:31 Arnd Bergmann
2016-10-28 15:50 ` Florian Westphal
0 siblings, 1 reply; 5+ messages in thread
From: Arnd Bergmann @ 2016-10-28 15:31 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Arnd Bergmann, Patrick McHardy, Jozsef Kadlecsik,
David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Florian Westphal, netfilter-devel, coreteam,
netdev, linux-kernel
The newly added nft fib code produces two warnings:
net/ipv4/netfilter/nft_fib_ipv4.c: In function 'nft_fib4_eval':
net/ipv4/netfilter/nft_fib_ipv4.c:80:6: error: unused variable 'i' [-Werror=unused-variable]
net/ipv4/netfilter/nft_fib_ipv4.c: In function ‘nft_fib4_eval’:
net/ipv4/netfilter/nft_fib_ipv4.c:137:6: error: ‘oif’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
The first one is obvious as the only user of that variable is
inside of an #ifdef, but the second one is a bit trickier.
It is clear that 'oif' is uninitialized here if neither
NFTA_FIB_F_OIF nor NFTA_FIB_F_IIF are set.
I have no idea how that should be handled, this patch just
returns without doing anything, which may or may not be
the right thing to do.
Fixes: 84f5eedb983e ("netfilter: nf_tables: add fib expression")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
net/ipv4/netfilter/nft_fib_ipv4.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/ipv4/netfilter/nft_fib_ipv4.c b/net/ipv4/netfilter/nft_fib_ipv4.c
index 6787c563cfc9..b29f70593e8b 100644
--- a/net/ipv4/netfilter/nft_fib_ipv4.c
+++ b/net/ipv4/netfilter/nft_fib_ipv4.c
@@ -77,7 +77,9 @@ void nft_fib4_eval(const struct nft_expr *expr, struct nft_regs *regs,
};
const struct net_device *oif;
struct net_device *found;
+#ifdef CONFIG_IP_ROUTE_MULTIPATH
int i;
+#endif
/*
* Do not set flowi4_oif, it restricts results (for example, asking
@@ -90,6 +92,8 @@ void nft_fib4_eval(const struct nft_expr *expr, struct nft_regs *regs,
oif = pkt->out;
else if (priv->flags & NFTA_FIB_F_IIF)
oif = pkt->in;
+ else
+ return;
if (pkt->hook == NF_INET_PRE_ROUTING && fib4_is_local(pkt->skb)) {
nft_fib_store_result(dest, priv->result, pkt, LOOPBACK_IFINDEX);
--
2.9.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] [rfc, netfilter-next] netfilter: nf_tables: fib warnings
2016-10-28 15:31 [PATCH] [rfc, netfilter-next] netfilter: nf_tables: fib warnings Arnd Bergmann
@ 2016-10-28 15:50 ` Florian Westphal
2016-10-28 16:15 ` Arnd Bergmann
0 siblings, 1 reply; 5+ messages in thread
From: Florian Westphal @ 2016-10-28 15:50 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik,
David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Florian Westphal, netfilter-devel, coreteam,
netdev, linux-kernel
Arnd Bergmann <arnd@arndb.de> wrote:
> The newly added nft fib code produces two warnings:
>
> net/ipv4/netfilter/nft_fib_ipv4.c: In function 'nft_fib4_eval':
> net/ipv4/netfilter/nft_fib_ipv4.c:80:6: error: unused variable 'i' [-Werror=unused-variable]
> net/ipv4/netfilter/nft_fib_ipv4.c: In function ‘nft_fib4_eval’:
> net/ipv4/netfilter/nft_fib_ipv4.c:137:6: error: ‘oif’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> The first one is obvious as the only user of that variable is
> inside of an #ifdef, but the second one is a bit trickier.
> It is clear that 'oif' is uninitialized here if neither
> NFTA_FIB_F_OIF nor NFTA_FIB_F_IIF are set.
>
> I have no idea how that should be handled, this patch just
> returns without doing anything, which may or may not be
> the right thing to do.
It should be initialized to NULL.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] [rfc, netfilter-next] netfilter: nf_tables: fib warnings
2016-10-28 15:50 ` Florian Westphal
@ 2016-10-28 16:15 ` Arnd Bergmann
2016-10-28 16:21 ` Florian Westphal
0 siblings, 1 reply; 5+ messages in thread
From: Arnd Bergmann @ 2016-10-28 16:15 UTC (permalink / raw)
To: Florian Westphal
Cc: Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik,
David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, netfilter-devel, coreteam, netdev,
linux-kernel
On Friday, October 28, 2016 5:50:31 PM CEST Florian Westphal wrote:
> Arnd Bergmann <arnd@arndb.de> wrote:
> > The newly added nft fib code produces two warnings:
> >
> > net/ipv4/netfilter/nft_fib_ipv4.c: In function 'nft_fib4_eval':
> > net/ipv4/netfilter/nft_fib_ipv4.c:80:6: error: unused variable 'i' [-Werror=unused-variable]
> > net/ipv4/netfilter/nft_fib_ipv4.c: In function ‘nft_fib4_eval’:
> > net/ipv4/netfilter/nft_fib_ipv4.c:137:6: error: ‘oif’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> >
> > The first one is obvious as the only user of that variable is
> > inside of an #ifdef, but the second one is a bit trickier.
> > It is clear that 'oif' is uninitialized here if neither
> > NFTA_FIB_F_OIF nor NFTA_FIB_F_IIF are set.
> >
> > I have no idea how that should be handled, this patch just
> > returns without doing anything, which may or may not be
> > the right thing to do.
>
> It should be initialized to NULL.
Ok, I had considered that, but wasn't sure if ->nh_dev could
ever be NULL, as that would then get dereferenced.
Arnd
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] [rfc, netfilter-next] netfilter: nf_tables: fib warnings
2016-10-28 16:15 ` Arnd Bergmann
@ 2016-10-28 16:21 ` Florian Westphal
2016-10-28 20:23 ` Arnd Bergmann
0 siblings, 1 reply; 5+ messages in thread
From: Florian Westphal @ 2016-10-28 16:21 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Florian Westphal, Pablo Neira Ayuso, Patrick McHardy,
Jozsef Kadlecsik, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, netfilter-devel, coreteam,
netdev, linux-kernel
Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday, October 28, 2016 5:50:31 PM CEST Florian Westphal wrote:
> > Arnd Bergmann <arnd@arndb.de> wrote:
> > > The newly added nft fib code produces two warnings:
> > >
> > > net/ipv4/netfilter/nft_fib_ipv4.c: In function 'nft_fib4_eval':
> > > net/ipv4/netfilter/nft_fib_ipv4.c:80:6: error: unused variable 'i' [-Werror=unused-variable]
> > > net/ipv4/netfilter/nft_fib_ipv4.c: In function ‘nft_fib4_eval’:
> > > net/ipv4/netfilter/nft_fib_ipv4.c:137:6: error: ‘oif’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > >
> > > The first one is obvious as the only user of that variable is
> > > inside of an #ifdef, but the second one is a bit trickier.
> > > It is clear that 'oif' is uninitialized here if neither
> > > NFTA_FIB_F_OIF nor NFTA_FIB_F_IIF are set.
> > >
> > > I have no idea how that should be handled, this patch just
> > > returns without doing anything, which may or may not be
> > > the right thing to do.
> >
> > It should be initialized to NULL.
>
> Ok, I had considered that, but wasn't sure if ->nh_dev could
> ever be NULL, as that would then get dereferenced.
Good point. In case oif is NULL we don't have to search the result
list for a match anyway, so we could do this (not even build tested):
diff --git a/net/ipv4/netfilter/nft_fib_ipv4.c b/net/ipv4/netfilter/nft_fib_ipv4.c
--- a/net/ipv4/netfilter/nft_fib_ipv4.c
+++ b/net/ipv4/netfilter/nft_fib_ipv4.c
@@ -130,6 +130,11 @@ void nft_fib4_eval(const struct nft_expr *expr, struct nft_regs *regs,
break;
}
+ if (!oif) {
+ found = FIB_RES_DEV(res);
+ goto ok;
+ }
+
#ifdef CONFIG_IP_ROUTE_MULTIPATH
for (i = 0; i < res.fi->fib_nhs; i++) {
struct fib_nh *nh = &res.fi->fib_nh[i];
@@ -139,16 +144,12 @@ void nft_fib4_eval(const struct nft_expr *expr, struct nft_regs *regs,
goto ok;
}
}
-#endif
- if (priv->flags & NFTA_FIB_F_OIF) {
- found = FIB_RES_DEV(res);
- if (found == oif)
- goto ok;
- return;
- }
-
- *dest = FIB_RES_DEV(res)->ifindex;
return;
+#else
+ found = FIB_RES_DEV(res);
+ if (found != oif)
+ return;
+#endif
ok:
switch (priv->result) {
I can take care of this as a followup.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] [rfc, netfilter-next] netfilter: nf_tables: fib warnings
2016-10-28 16:21 ` Florian Westphal
@ 2016-10-28 20:23 ` Arnd Bergmann
0 siblings, 0 replies; 5+ messages in thread
From: Arnd Bergmann @ 2016-10-28 20:23 UTC (permalink / raw)
To: Florian Westphal
Cc: Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik,
David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, netfilter-devel, coreteam, netdev,
linux-kernel
On Friday, October 28, 2016 6:21:49 PM CEST Florian Westphal wrote:
> Good point. In case oif is NULL we don't have to search the result
> list for a match anyway, so we could do this (not even build tested):
>
It didn't apply cleanly, but I've integrated it with the change to
initialize oif to NULL and the added #ifdef I had in my first version
and got a clean build.
I sent out a v2 now, but didn't try hard to understand your changes.
Arnd
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-10-28 20:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-28 15:31 [PATCH] [rfc, netfilter-next] netfilter: nf_tables: fib warnings Arnd Bergmann
2016-10-28 15:50 ` Florian Westphal
2016-10-28 16:15 ` Arnd Bergmann
2016-10-28 16:21 ` Florian Westphal
2016-10-28 20:23 ` Arnd Bergmann
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).