* [PATCH net] ipv4: initialize flowi4_flags before calling fib_lookup()
@ 2016-03-22 15:31 Lance Richardson
2016-03-22 18:29 ` David Ahern
0 siblings, 1 reply; 12+ messages in thread
From: Lance Richardson @ 2016-03-22 15:31 UTC (permalink / raw)
To: netdev
Field fl4.flowi4_flags is not initialized in fib_compute_spec_dst()
before calling fib_lookup(), which means fib_table_lookup() is
using non-deterministic data at this line:
if (!(flp->flowi4_flags & FLOWI_FLAG_SKIP_NH_OIF)) {
Fix by initializing fl4.flowi4_flags to zero.
Signed-off-by: Lance Richardson <lrichard@redhat.com>
---
net/ipv4/fib_frontend.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index 21add55..896844a 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -304,6 +304,7 @@ __be32 fib_compute_spec_dst(struct sk_buff *skb)
fl4.flowi4_scope = scope;
fl4.flowi4_mark = IN_DEV_SRC_VMARK(in_dev) ? skb->mark : 0;
fl4.flowi4_tun_key.tun_id = 0;
+ fl4.flowi4_flags = 0;
if (!fib_lookup(net, &fl4, &res, 0))
return FIB_RES_PREFSRC(net, res);
} else {
--
2.5.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH net] ipv4: initialize flowi4_flags before calling fib_lookup()
2016-03-22 15:31 Lance Richardson
@ 2016-03-22 18:29 ` David Ahern
2016-03-22 18:41 ` Lance Richardson
2016-03-22 20:45 ` Cong Wang
0 siblings, 2 replies; 12+ messages in thread
From: David Ahern @ 2016-03-22 18:29 UTC (permalink / raw)
To: Lance Richardson, netdev
On 3/22/16 9:31 AM, Lance Richardson wrote:
> Field fl4.flowi4_flags is not initialized in fib_compute_spec_dst()
> before calling fib_lookup(), which means fib_table_lookup() is
> using non-deterministic data at this line:
>
> if (!(flp->flowi4_flags & FLOWI_FLAG_SKIP_NH_OIF)) {
>
> Fix by initializing fl4.flowi4_flags to zero.
>
> Signed-off-by: Lance Richardson <lrichard@redhat.com>
> ---
> net/ipv4/fib_frontend.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
> index 21add55..896844a 100644
> --- a/net/ipv4/fib_frontend.c
> +++ b/net/ipv4/fib_frontend.c
> @@ -304,6 +304,7 @@ __be32 fib_compute_spec_dst(struct sk_buff *skb)
> fl4.flowi4_scope = scope;
> fl4.flowi4_mark = IN_DEV_SRC_VMARK(in_dev) ? skb->mark : 0;
> fl4.flowi4_tun_key.tun_id = 0;
> + fl4.flowi4_flags = 0;
> if (!fib_lookup(net, &fl4, &res, 0))
> return FIB_RES_PREFSRC(net, res);
> } else {
>
Fixes: 58189ca7b2741 ("net: Fix vti use case with oif in dst lookups")
I think a more robust solution is to move fl4 to this if case and init
when it is declared:
struct flowi4 fl4 = {
.flowi4_iif = LOOPBACK_IFINDEX,
.daddr = ip_hdr(skb)->saddr,
.flowi4_tos = RT_TOS(ip_hdr(skb)->tos),
.flowi4_scope = scope,
.flowi4_mark = IN_DEV_SRC_VMARK(in_dev) ? skb->mark : 0,
};
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net] ipv4: initialize flowi4_flags before calling fib_lookup()
2016-03-22 18:29 ` David Ahern
@ 2016-03-22 18:41 ` Lance Richardson
2016-03-22 20:45 ` Cong Wang
1 sibling, 0 replies; 12+ messages in thread
From: Lance Richardson @ 2016-03-22 18:41 UTC (permalink / raw)
To: David Ahern; +Cc: netdev
----- Original Message -----
> From: "David Ahern" <dsa@cumulusnetworks.com>
> To: "Lance Richardson" <lrichard@redhat.com>, netdev@vger.kernel.org
> Sent: Tuesday, March 22, 2016 2:29:02 PM
> Subject: Re: [PATCH net] ipv4: initialize flowi4_flags before calling fib_lookup()
>
> On 3/22/16 9:31 AM, Lance Richardson wrote:
> > Field fl4.flowi4_flags is not initialized in fib_compute_spec_dst()
> > before calling fib_lookup(), which means fib_table_lookup() is
> > using non-deterministic data at this line:
> >
> > if (!(flp->flowi4_flags & FLOWI_FLAG_SKIP_NH_OIF)) {
> >
> > Fix by initializing fl4.flowi4_flags to zero.
> >
> > Signed-off-by: Lance Richardson <lrichard@redhat.com>
> > ---
> > net/ipv4/fib_frontend.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
> > index 21add55..896844a 100644
> > --- a/net/ipv4/fib_frontend.c
> > +++ b/net/ipv4/fib_frontend.c
> > @@ -304,6 +304,7 @@ __be32 fib_compute_spec_dst(struct sk_buff *skb)
> > fl4.flowi4_scope = scope;
> > fl4.flowi4_mark = IN_DEV_SRC_VMARK(in_dev) ? skb->mark : 0;
> > fl4.flowi4_tun_key.tun_id = 0;
> > + fl4.flowi4_flags = 0;
> > if (!fib_lookup(net, &fl4, &res, 0))
> > return FIB_RES_PREFSRC(net, res);
> > } else {
> >
>
> Fixes: 58189ca7b2741 ("net: Fix vti use case with oif in dst lookups")
>
> I think a more robust solution is to move fl4 to this if case and init
> when it is declared:
>
> struct flowi4 fl4 = {
> .flowi4_iif = LOOPBACK_IFINDEX,
> .daddr = ip_hdr(skb)->saddr,
> .flowi4_tos = RT_TOS(ip_hdr(skb)->tos),
> .flowi4_scope = scope,
> .flowi4_mark = IN_DEV_SRC_VMARK(in_dev) ? skb->mark : 0,
> };
>
Agreed... I actually debated doing something similar but opted for the
smaller delta.
v2 coming up.
Thanks for the review,
Lance
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH net] ipv4: initialize flowi4_flags before calling fib_lookup()
@ 2016-03-22 18:56 Lance Richardson
2016-03-22 18:58 ` Lance Richardson
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Lance Richardson @ 2016-03-22 18:56 UTC (permalink / raw)
To: netdev; +Cc: dsa
Field fl4.flowi4_flags is not initialized in fib_compute_spec_dst()
before calling fib_lookup(), which means fib_table_lookup() is
using non-deterministic data at this line:
if (!(flp->flowi4_flags & FLOWI_FLAG_SKIP_NH_OIF)) {
Fix by initializing the entire fl4 structure, which will prevent
similar issues as fields are added in the future by ensuring that
all fields are initialized to zero unless explicitly initialized
to another value.
Fixes: 58189ca7b2741 ("net: Fix vti use case with oif in dst lookups")
Suggested-by: David Ahern <dsa@cumulusnetworks.com>
Signed-off-by: Lance Richardson <lrichard@redhat.com>
---
net/ipv4/fib_frontend.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index 21add55..8a9246d 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -280,7 +280,6 @@ __be32 fib_compute_spec_dst(struct sk_buff *skb)
struct in_device *in_dev;
struct fib_result res;
struct rtable *rt;
- struct flowi4 fl4;
struct net *net;
int scope;
@@ -296,14 +295,13 @@ __be32 fib_compute_spec_dst(struct sk_buff *skb)
scope = RT_SCOPE_UNIVERSE;
if (!ipv4_is_zeronet(ip_hdr(skb)->saddr)) {
- fl4.flowi4_oif = 0;
- fl4.flowi4_iif = LOOPBACK_IFINDEX;
- fl4.daddr = ip_hdr(skb)->saddr;
- fl4.saddr = 0;
- fl4.flowi4_tos = RT_TOS(ip_hdr(skb)->tos);
- fl4.flowi4_scope = scope;
- fl4.flowi4_mark = IN_DEV_SRC_VMARK(in_dev) ? skb->mark : 0;
- fl4.flowi4_tun_key.tun_id = 0;
+ struct flowi4 fl4 = {
+ .flowi4_iif = LOOPBACK_IFINDEX,
+ .daddr = ip_hdr(skb)->saddr,
+ .flowi4_tos = RT_TOS(ip_hdr(skb)->tos),
+ .flowi4_scope = scope,
+ .flowi4_mark = IN_DEV_SRC_VMARK(in_dev) ? skb->mark : 0,
+ };
if (!fib_lookup(net, &fl4, &res, 0))
return FIB_RES_PREFSRC(net, res);
} else {
--
2.5.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH net] ipv4: initialize flowi4_flags before calling fib_lookup()
2016-03-22 18:56 [PATCH net] ipv4: initialize flowi4_flags before calling fib_lookup() Lance Richardson
@ 2016-03-22 18:58 ` Lance Richardson
2016-03-22 20:03 ` David Miller
2016-03-22 19:00 ` David Ahern
2016-03-23 11:35 ` Sergei Shtylyov
2 siblings, 1 reply; 12+ messages in thread
From: Lance Richardson @ 2016-03-22 18:58 UTC (permalink / raw)
To: netdev; +Cc: dsa
Apologies, that should have been [PATCH v2 net].
----- Original Message -----
> From: "Lance Richardson" <lrichard@redhat.com>
> To: netdev@vger.kernel.org
> Cc: dsa@cumulusnetworks.com
> Sent: Tuesday, March 22, 2016 2:56:57 PM
> Subject: [PATCH net] ipv4: initialize flowi4_flags before calling fib_lookup()
>
> Field fl4.flowi4_flags is not initialized in fib_compute_spec_dst()
> before calling fib_lookup(), which means fib_table_lookup() is
> using non-deterministic data at this line:
>
> if (!(flp->flowi4_flags & FLOWI_FLAG_SKIP_NH_OIF)) {
>
> Fix by initializing the entire fl4 structure, which will prevent
> similar issues as fields are added in the future by ensuring that
> all fields are initialized to zero unless explicitly initialized
> to another value.
>
> Fixes: 58189ca7b2741 ("net: Fix vti use case with oif in dst lookups")
> Suggested-by: David Ahern <dsa@cumulusnetworks.com>
> Signed-off-by: Lance Richardson <lrichard@redhat.com>
> ---
> net/ipv4/fib_frontend.c | 16 +++++++---------
> 1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
> index 21add55..8a9246d 100644
> --- a/net/ipv4/fib_frontend.c
> +++ b/net/ipv4/fib_frontend.c
> @@ -280,7 +280,6 @@ __be32 fib_compute_spec_dst(struct sk_buff *skb)
> struct in_device *in_dev;
> struct fib_result res;
> struct rtable *rt;
> - struct flowi4 fl4;
> struct net *net;
> int scope;
>
> @@ -296,14 +295,13 @@ __be32 fib_compute_spec_dst(struct sk_buff *skb)
>
> scope = RT_SCOPE_UNIVERSE;
> if (!ipv4_is_zeronet(ip_hdr(skb)->saddr)) {
> - fl4.flowi4_oif = 0;
> - fl4.flowi4_iif = LOOPBACK_IFINDEX;
> - fl4.daddr = ip_hdr(skb)->saddr;
> - fl4.saddr = 0;
> - fl4.flowi4_tos = RT_TOS(ip_hdr(skb)->tos);
> - fl4.flowi4_scope = scope;
> - fl4.flowi4_mark = IN_DEV_SRC_VMARK(in_dev) ? skb->mark : 0;
> - fl4.flowi4_tun_key.tun_id = 0;
> + struct flowi4 fl4 = {
> + .flowi4_iif = LOOPBACK_IFINDEX,
> + .daddr = ip_hdr(skb)->saddr,
> + .flowi4_tos = RT_TOS(ip_hdr(skb)->tos),
> + .flowi4_scope = scope,
> + .flowi4_mark = IN_DEV_SRC_VMARK(in_dev) ? skb->mark : 0,
> + };
> if (!fib_lookup(net, &fl4, &res, 0))
> return FIB_RES_PREFSRC(net, res);
> } else {
> --
> 2.5.5
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net] ipv4: initialize flowi4_flags before calling fib_lookup()
2016-03-22 18:56 [PATCH net] ipv4: initialize flowi4_flags before calling fib_lookup() Lance Richardson
2016-03-22 18:58 ` Lance Richardson
@ 2016-03-22 19:00 ` David Ahern
2016-03-22 20:15 ` Eric Dumazet
2016-03-23 11:35 ` Sergei Shtylyov
2 siblings, 1 reply; 12+ messages in thread
From: David Ahern @ 2016-03-22 19:00 UTC (permalink / raw)
To: Lance Richardson, netdev
On 3/22/16 12:56 PM, Lance Richardson wrote:
> Field fl4.flowi4_flags is not initialized in fib_compute_spec_dst()
> before calling fib_lookup(), which means fib_table_lookup() is
> using non-deterministic data at this line:
>
> if (!(flp->flowi4_flags & FLOWI_FLAG_SKIP_NH_OIF)) {
>
> Fix by initializing the entire fl4 structure, which will prevent
> similar issues as fields are added in the future by ensuring that
> all fields are initialized to zero unless explicitly initialized
> to another value.
>
> Fixes: 58189ca7b2741 ("net: Fix vti use case with oif in dst lookups")
> Suggested-by: David Ahern <dsa@cumulusnetworks.com>
> Signed-off-by: Lance Richardson <lrichard@redhat.com>
> ---
> net/ipv4/fib_frontend.c | 16 +++++++---------
> 1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
> index 21add55..8a9246d 100644
> --- a/net/ipv4/fib_frontend.c
> +++ b/net/ipv4/fib_frontend.c
> @@ -280,7 +280,6 @@ __be32 fib_compute_spec_dst(struct sk_buff *skb)
> struct in_device *in_dev;
> struct fib_result res;
> struct rtable *rt;
> - struct flowi4 fl4;
> struct net *net;
> int scope;
>
> @@ -296,14 +295,13 @@ __be32 fib_compute_spec_dst(struct sk_buff *skb)
>
> scope = RT_SCOPE_UNIVERSE;
> if (!ipv4_is_zeronet(ip_hdr(skb)->saddr)) {
> - fl4.flowi4_oif = 0;
> - fl4.flowi4_iif = LOOPBACK_IFINDEX;
> - fl4.daddr = ip_hdr(skb)->saddr;
> - fl4.saddr = 0;
> - fl4.flowi4_tos = RT_TOS(ip_hdr(skb)->tos);
> - fl4.flowi4_scope = scope;
> - fl4.flowi4_mark = IN_DEV_SRC_VMARK(in_dev) ? skb->mark : 0;
> - fl4.flowi4_tun_key.tun_id = 0;
> + struct flowi4 fl4 = {
> + .flowi4_iif = LOOPBACK_IFINDEX,
> + .daddr = ip_hdr(skb)->saddr,
> + .flowi4_tos = RT_TOS(ip_hdr(skb)->tos),
> + .flowi4_scope = scope,
> + .flowi4_mark = IN_DEV_SRC_VMARK(in_dev) ? skb->mark : 0,
> + };
> if (!fib_lookup(net, &fl4, &res, 0))
> return FIB_RES_PREFSRC(net, res);
> } else {
>
Acked-by: David Ahern <dsa@cumulusnetworks.com>
DaveM: this should go into stable releases back to v4.3.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net] ipv4: initialize flowi4_flags before calling fib_lookup()
2016-03-22 18:58 ` Lance Richardson
@ 2016-03-22 20:03 ` David Miller
0 siblings, 0 replies; 12+ messages in thread
From: David Miller @ 2016-03-22 20:03 UTC (permalink / raw)
To: lrichard; +Cc: netdev, dsa
From: Lance Richardson <lrichard@redhat.com>
Date: Tue, 22 Mar 2016 14:58:59 -0400 (EDT)
> Apologies, that should have been [PATCH v2 net].
No worries.
Applied and queued up for -stable, thanks.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net] ipv4: initialize flowi4_flags before calling fib_lookup()
2016-03-22 19:00 ` David Ahern
@ 2016-03-22 20:15 ` Eric Dumazet
0 siblings, 0 replies; 12+ messages in thread
From: Eric Dumazet @ 2016-03-22 20:15 UTC (permalink / raw)
To: David Ahern; +Cc: Lance Richardson, netdev
On Tue, 2016-03-22 at 13:00 -0600, David Ahern wrote:
> On 3/22/16 12:56 PM, Lance Richardson wrote:
> >
> > Fixes: 58189ca7b2741 ("net: Fix vti use case with oif in dst lookups")
> DaveM: this should go into stable releases back to v4.3.
>
The 'Fixes' tag tells this already ;)
$ git describe --contains 58189ca7b2741
v4.3-rc3~13^2~63
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net] ipv4: initialize flowi4_flags before calling fib_lookup()
2016-03-22 18:29 ` David Ahern
2016-03-22 18:41 ` Lance Richardson
@ 2016-03-22 20:45 ` Cong Wang
2016-03-22 21:02 ` David Ahern
1 sibling, 1 reply; 12+ messages in thread
From: Cong Wang @ 2016-03-22 20:45 UTC (permalink / raw)
To: David Ahern; +Cc: Lance Richardson, Linux Kernel Network Developers
On Tue, Mar 22, 2016 at 11:29 AM, David Ahern <dsa@cumulusnetworks.com> wrote:
> On 3/22/16 9:31 AM, Lance Richardson wrote:
>>
>> Field fl4.flowi4_flags is not initialized in fib_compute_spec_dst()
>> before calling fib_lookup(), which means fib_table_lookup() is
>> using non-deterministic data at this line:
>>
>> if (!(flp->flowi4_flags & FLOWI_FLAG_SKIP_NH_OIF)) {
>>
>> Fix by initializing fl4.flowi4_flags to zero.
>>
>> Signed-off-by: Lance Richardson <lrichard@redhat.com>
>> ---
>> net/ipv4/fib_frontend.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
>> index 21add55..896844a 100644
>> --- a/net/ipv4/fib_frontend.c
>> +++ b/net/ipv4/fib_frontend.c
>> @@ -304,6 +304,7 @@ __be32 fib_compute_spec_dst(struct sk_buff *skb)
>> fl4.flowi4_scope = scope;
>> fl4.flowi4_mark = IN_DEV_SRC_VMARK(in_dev) ? skb->mark :
>> 0;
>> fl4.flowi4_tun_key.tun_id = 0;
>> + fl4.flowi4_flags = 0;
>> if (!fib_lookup(net, &fl4, &res, 0))
>> return FIB_RES_PREFSRC(net, res);
>> } else {
>>
>
> Fixes: 58189ca7b2741 ("net: Fix vti use case with oif in dst lookups")
Why does this patch fix this commit? Even before this commit, flowi4_flags
was already tested for other bit:
@@ -1426,7 +1426,7 @@ int fib_table_lookup(struct fib_table *tb, const
struct flowi4 *flp,
nh->nh_flags & RTNH_F_LINKDOWN &&
!(fib_flags & FIB_LOOKUP_IGNORE_LINKSTATE))
continue;
- if (!(flp->flowi4_flags & FLOWI_FLAG_VRFSRC)) {
+ if (!(flp->flowi4_flags & FLOWI_FLAG_SKIP_NH_OIF)) {
if (flp->flowi4_oif &&
flp->flowi4_oif != nh->nh_oif)
continue;
For me, it looks the bug was introduce by commit 35ebf65e851c6d9731abc6362b1.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net] ipv4: initialize flowi4_flags before calling fib_lookup()
2016-03-22 20:45 ` Cong Wang
@ 2016-03-22 21:02 ` David Ahern
2016-03-22 21:10 ` Cong Wang
0 siblings, 1 reply; 12+ messages in thread
From: David Ahern @ 2016-03-22 21:02 UTC (permalink / raw)
To: Cong Wang; +Cc: Lance Richardson, Linux Kernel Network Developers
On 3/22/16 2:45 PM, Cong Wang wrote:
> @@ -1426,7 +1426,7 @@ int fib_table_lookup(struct fib_table *tb, const
> struct flowi4 *flp,
> nh->nh_flags & RTNH_F_LINKDOWN &&
> !(fib_flags & FIB_LOOKUP_IGNORE_LINKSTATE))
> continue;
> - if (!(flp->flowi4_flags & FLOWI_FLAG_VRFSRC)) {
> + if (!(flp->flowi4_flags & FLOWI_FLAG_SKIP_NH_OIF)) {
> if (flp->flowi4_oif &&
> flp->flowi4_oif != nh->nh_oif)
> continue;
>
> For me, it looks the bug was introduce by commit 35ebf65e851c6d9731abc6362b1.
>
Arguably yes since it added the function without initializing flags.
The commit I referenced (and even the VRF predecessor both of which
originated in the v4.3) is the one introducing use of flow flags to the
lookup. From a stable perspective going back to v4.3 is what matters.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net] ipv4: initialize flowi4_flags before calling fib_lookup()
2016-03-22 21:02 ` David Ahern
@ 2016-03-22 21:10 ` Cong Wang
0 siblings, 0 replies; 12+ messages in thread
From: Cong Wang @ 2016-03-22 21:10 UTC (permalink / raw)
To: David Ahern; +Cc: Lance Richardson, Linux Kernel Network Developers
On Tue, Mar 22, 2016 at 2:02 PM, David Ahern <dsa@cumulusnetworks.com> wrote:
> On 3/22/16 2:45 PM, Cong Wang wrote:
>>
>> @@ -1426,7 +1426,7 @@ int fib_table_lookup(struct fib_table *tb, const
>> struct flowi4 *flp,
>> nh->nh_flags & RTNH_F_LINKDOWN &&
>> !(fib_flags & FIB_LOOKUP_IGNORE_LINKSTATE))
>> continue;
>> - if (!(flp->flowi4_flags & FLOWI_FLAG_VRFSRC)) {
>> + if (!(flp->flowi4_flags & FLOWI_FLAG_SKIP_NH_OIF))
>> {
>> if (flp->flowi4_oif &&
>> flp->flowi4_oif != nh->nh_oif)
>> continue;
>>
>> For me, it looks the bug was introduce by commit
>> 35ebf65e851c6d9731abc6362b1.
>>
>
> Arguably yes since it added the function without initializing flags.
>
> The commit I referenced (and even the VRF predecessor both of which
> originated in the v4.3) is the one introducing use of flow flags to the
> lookup. From a stable perspective going back to v4.3 is what matters.
>
Yeah, then it is commit 613d09b30f8b589d5a9b497. It doesn't matter
for backport, but matters for accuracy. ;)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net] ipv4: initialize flowi4_flags before calling fib_lookup()
2016-03-22 18:56 [PATCH net] ipv4: initialize flowi4_flags before calling fib_lookup() Lance Richardson
2016-03-22 18:58 ` Lance Richardson
2016-03-22 19:00 ` David Ahern
@ 2016-03-23 11:35 ` Sergei Shtylyov
2 siblings, 0 replies; 12+ messages in thread
From: Sergei Shtylyov @ 2016-03-23 11:35 UTC (permalink / raw)
To: Lance Richardson, netdev; +Cc: dsa
Hello.
On 3/22/2016 9:56 PM, Lance Richardson wrote:
> Field fl4.flowi4_flags is not initialized in fib_compute_spec_dst()
> before calling fib_lookup(), which means fib_table_lookup() is
> using non-deterministic data at this line:
>
> if (!(flp->flowi4_flags & FLOWI_FLAG_SKIP_NH_OIF)) {
>
> Fix by initializing the entire fl4 structure, which will prevent
> similar issues as fields are added in the future by ensuring that
> all fields are initialized to zero unless explicitly initialized
> to another value.
>
> Fixes: 58189ca7b2741 ("net: Fix vti use case with oif in dst lookups")
> Suggested-by: David Ahern <dsa@cumulusnetworks.com>
> Signed-off-by: Lance Richardson <lrichard@redhat.com>
> ---
> net/ipv4/fib_frontend.c | 16 +++++++---------
> 1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
> index 21add55..8a9246d 100644
> --- a/net/ipv4/fib_frontend.c
> +++ b/net/ipv4/fib_frontend.c
[...]
> @@ -296,14 +295,13 @@ __be32 fib_compute_spec_dst(struct sk_buff *skb)
>
> scope = RT_SCOPE_UNIVERSE;
> if (!ipv4_is_zeronet(ip_hdr(skb)->saddr)) {
> - fl4.flowi4_oif = 0;
> - fl4.flowi4_iif = LOOPBACK_IFINDEX;
> - fl4.daddr = ip_hdr(skb)->saddr;
> - fl4.saddr = 0;
> - fl4.flowi4_tos = RT_TOS(ip_hdr(skb)->tos);
> - fl4.flowi4_scope = scope;
> - fl4.flowi4_mark = IN_DEV_SRC_VMARK(in_dev) ? skb->mark : 0;
> - fl4.flowi4_tun_key.tun_id = 0;
> + struct flowi4 fl4 = {
> + .flowi4_iif = LOOPBACK_IFINDEX,
> + .daddr = ip_hdr(skb)->saddr,
> + .flowi4_tos = RT_TOS(ip_hdr(skb)->tos),
> + .flowi4_scope = scope,
> + .flowi4_mark = IN_DEV_SRC_VMARK(in_dev) ? skb->mark : 0,
> + };
Need empty line after the declaration.
> if (!fib_lookup(net, &fl4, &res, 0))
> return FIB_RES_PREFSRC(net, res);
> } else {
MBR, Sergei
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-03-23 11:35 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-22 18:56 [PATCH net] ipv4: initialize flowi4_flags before calling fib_lookup() Lance Richardson
2016-03-22 18:58 ` Lance Richardson
2016-03-22 20:03 ` David Miller
2016-03-22 19:00 ` David Ahern
2016-03-22 20:15 ` Eric Dumazet
2016-03-23 11:35 ` Sergei Shtylyov
-- strict thread matches above, loose matches on Subject: below --
2016-03-22 15:31 Lance Richardson
2016-03-22 18:29 ` David Ahern
2016-03-22 18:41 ` Lance Richardson
2016-03-22 20:45 ` Cong Wang
2016-03-22 21:02 ` David Ahern
2016-03-22 21:10 ` Cong Wang
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).