[PATCH] net-ipv6: fix excessive RTF_ADDRCONF flag on ::1/128 local route (and others)

[PATCH] net-ipv6: fix excessive RTF_ADDRCONF flag on ::1/128 local route (and others)

[Maciej Żenczykowski]

From: Maciej Żenczykowski <maze@google.com>

There is a subtle change in behaviour introduced by:
  commit c7a1ce397adacaf5d4bb2eab0a738b5f80dc3e43
  'ipv6: Change addrconf_f6i_alloc to use ip6_route_info_create'

Before that patch /proc/net/ipv6_route includes:
00000000000000000000000000000001 80 00000000000000000000000000000000 00 00000000000000000000000000000000 00000000 00000003 00000000 80200001 lo

Afterwards /proc/net/ipv6_route includes:
00000000000000000000000000000001 80 00000000000000000000000000000000 00 00000000000000000000000000000000 00000000 00000002 00000000 80240001 lo

ie. the above commit causes the ::/128 local (automatic) route to be flagged with RTF_ADDRCONF (0x040000).

AFAICT, this is incorrect since these routes are *not* coming from RA's.

As such, this patch restores the old behaviour.

Fixes: c7a1ce397adacaf5d4bb2eab0a738b5f80dc3e43
Cc: David Ahern <dsahern@gmail.com>
Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 net/ipv6/route.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 558c6c68855f..cee977e52d34 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -4365,13 +4365,14 @@ struct fib6_info *addrconf_f6i_alloc(struct net *net,
 	struct fib6_config cfg = {
 		.fc_table = l3mdev_fib_table(idev->dev) ? : RT6_TABLE_LOCAL,
 		.fc_ifindex = idev->dev->ifindex,
-		.fc_flags = RTF_UP | RTF_ADDRCONF | RTF_NONEXTHOP,
+		.fc_flags = RTF_UP | RTF_NONEXTHOP,
 		.fc_dst = *addr,
 		.fc_dst_len = 128,
 		.fc_protocol = RTPROT_KERNEL,
 		.fc_nlinfo.nl_net = net,
 		.fc_ignore_dev_down = true,
 	};
+	struct fib6_info *f6i;
 
 	if (anycast) {
 		cfg.fc_type = RTN_ANYCAST;
@@ -4381,7 +4382,9 @@ struct fib6_info *addrconf_f6i_alloc(struct net *net,
 		cfg.fc_flags |= RTF_LOCAL;
 	}
 
-	return ip6_route_info_create(&cfg, gfp_flags, NULL);
+	f6i = ip6_route_info_create(&cfg, gfp_flags, NULL);
+	f6i->dst_nocount = true;
+	return f6i;
 }
 
 /* remove deleted ip from prefsrc entries */
-- 
2.23.0.187.g17f5b7556c-goog

Re: [PATCH] net-ipv6: fix excessive RTF_ADDRCONF flag on ::1/128 local route (and others)

[Maciej Żenczykowski]

Some background:

This was found due to bad interactions with one of the few remaining
Android common kernel networking patches.
(The one that makes it possible for RA's to create routes in interface
specific tables)

The cleanup portion of it scours all tables and deletes all relevant
ADDRCONF routes, which in 5.2-rc1+ now includes ::1/128 and thus
terribly breaks things (in the Android Kernel Networking tests).

However, it *is* a userspace visible change in behaviour (since it's
visible via the above /proc file),
so one could argue for the above patch (or something similar).

The Android patch *could* also probably be adjusted to handle this
case (and thus prevent the breakage).

It's not immediately clear to me what is the better approach as I'm
not immediately certain what RTF_ADDRCONF truly means.
However the in kernel header file comment does explicitly mention this
being used to flag routes derived from RA's, and very clearly ::1/128
is not RA generated, so I *think* the correct fix is to return to the
old way the kernel used to do things and not flag with ADDRCONF...

Opinions?

Re: [PATCH] net-ipv6: fix excessive RTF_ADDRCONF flag on ::1/128 local route (and others)

[Lorenzo Colitti]

On Mon, Sep 2, 2019 at 2:55 AM Maciej Żenczykowski
<zenczykowski@gmail.com> wrote:
> It's not immediately clear to me what is the better approach as I'm
> not immediately certain what RTF_ADDRCONF truly means.
> However the in kernel header file comment does explicitly mention this
> being used to flag routes derived from RA's, and very clearly ::1/128
> is not RA generated, so I *think* the correct fix is to return to the
> old way the kernel used to do things and not flag with ADDRCONF...

AIUI, "addrconf" has always meant stateless address autoconfiguration
as per RFC 4862, i.e., addresses autoconfigured when getting an RA, or
autoconfigured based on adding the link-local prefix. Looking at 5.1
(the most recent release before c7a1ce397ada which you're fixing here)
confirms this interpretation, because RTF_ADDRCONF is only used by:

- addrconf_prefix_rcv: receiving a PIO from an RA
- rt6_add_route_info: receiving an RIO from an RA
- rt6_add_dflt_router, rt6_get_dflt_router: receiving the default
router from an RA
- __rt6_purge_dflt_routers: removing all routes received from RAs,
when enabling forwarding (i.e., switching from being a host to being a
router)


So, if I'm reading c7a1ce397ada right, I would say it's incorrect.
That patch changes things so that RTF_ADDRCONF is set for pretty much
all routes created by adding IPv6 addresses. That includes not only
IPv6 addresses created by RAs, which has always been the case, but
also IPv6 addresses created manually from userspace, or the loopback
address, and even multicast and anycast addresses created by
IPV6_JOIN_GROUP and IPV6_JOIN_ANYCAST. That's userspace-visible
breakage and should be reverted.

Not sure if this patch is the right fix, though, because it breaks
things in the opposite direction: even routes created by an IPv6
address added by receiving an RA will no longer have RTF_ADDRCONF.
Perhaps add something like this as well?

 struct fib6_info *addrconf_f6i_alloc(struct net *net, struct inet6_dev *idev,
-                                     const struct in6_addr *addr, bool anycast,
-                                     const struct in6_addr *addr, u8 flags,
                                      gfp_t gfp_flags);

flags would be RTF_ANYCAST iff the code previously called with true,
and RTF_ADDRCONF if called by a function that is adding an IPv6
address coming from an RA.

Re: [PATCH] net-ipv6: fix excessive RTF_ADDRCONF flag on ::1/128 local route (and others)

[David Ahern]

On 9/1/19 11:47 AM, Maciej Żenczykowski wrote:
> From: Maciej Żenczykowski <maze@google.com>
> 
> There is a subtle change in behaviour introduced by:
>   commit c7a1ce397adacaf5d4bb2eab0a738b5f80dc3e43
>   'ipv6: Change addrconf_f6i_alloc to use ip6_route_info_create'
> 
> Before that patch /proc/net/ipv6_route includes:
> 00000000000000000000000000000001 80 00000000000000000000000000000000 00 00000000000000000000000000000000 00000000 00000003 00000000 80200001 lo
> 
> Afterwards /proc/net/ipv6_route includes:
> 00000000000000000000000000000001 80 00000000000000000000000000000000 00 00000000000000000000000000000000 00000000 00000002 00000000 80240001 lo
> 
> ie. the above commit causes the ::/128 local (automatic) route to be flagged with RTF_ADDRCONF (0x040000).

The ADDRCONF flag is wrong.

> 
> AFAICT, this is incorrect since these routes are *not* coming from RA's.
> 
> As such, this patch restores the old behaviour.
> 
> Fixes: c7a1ce397adacaf5d4bb2eab0a738b5f80dc3e43
> Cc: David Ahern <dsahern@gmail.com>
> Signed-off-by: Maciej Żenczykowski <maze@google.com>
> ---
>  net/ipv6/route.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 558c6c68855f..cee977e52d34 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -4365,13 +4365,14 @@ struct fib6_info *addrconf_f6i_alloc(struct net *net,
>  	struct fib6_config cfg = {
>  		.fc_table = l3mdev_fib_table(idev->dev) ? : RT6_TABLE_LOCAL,
>  		.fc_ifindex = idev->dev->ifindex,
> -		.fc_flags = RTF_UP | RTF_ADDRCONF | RTF_NONEXTHOP,
> +		.fc_flags = RTF_UP | RTF_NONEXTHOP,
>  		.fc_dst = *addr,
>  		.fc_dst_len = 128,
>  		.fc_protocol = RTPROT_KERNEL,
>  		.fc_nlinfo.nl_net = net,
>  		.fc_ignore_dev_down = true,
>  	};
> +	struct fib6_info *f6i;
>  
>  	if (anycast) {
>  		cfg.fc_type = RTN_ANYCAST;
> @@ -4381,7 +4382,9 @@ struct fib6_info *addrconf_f6i_alloc(struct net *net,
>  		cfg.fc_flags |= RTF_LOCAL;
>  	}
>  
> -	return ip6_route_info_create(&cfg, gfp_flags, NULL);
> +	f6i = ip6_route_info_create(&cfg, gfp_flags, NULL);

ip6_route_info_create can return NULL.



> +	f6i->dst_nocount = true;
> +	return f6i;
>  }
>  
>  /* remove deleted ip from prefsrc entries */
> 

[PATCH v2] net-ipv6: fix excessive RTF_ADDRCONF flag on ::1/128 local route (and others)

[Maciej Żenczykowski]

From: Maciej Żenczykowski <maze@google.com>

There is a subtle change in behaviour introduced by:
  commit c7a1ce397adacaf5d4bb2eab0a738b5f80dc3e43
  'ipv6: Change addrconf_f6i_alloc to use ip6_route_info_create'

Before that patch /proc/net/ipv6_route includes:
00000000000000000000000000000001 80 00000000000000000000000000000000 00 00000000000000000000000000000000 00000000 00000003 00000000 80200001 lo

Afterwards /proc/net/ipv6_route includes:
00000000000000000000000000000001 80 00000000000000000000000000000000 00 00000000000000000000000000000000 00000000 00000002 00000000 80240001 lo

ie. the above commit causes the ::1/128 local (automatic) route to be flagged with RTF_ADDRCONF (0x040000).

AFAICT, this is incorrect since these routes are *not* coming from RA's.

As such, this patch restores the old behaviour.

Fixes: c7a1ce397adacaf5d4bb2eab0a738b5f80dc3e43
Cc: David Ahern <dsahern@gmail.com>
Cc: Lorenzo Colitti <lorenzo@google.com>
Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 net/ipv6/route.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 558c6c68855f..516b2e568dae 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -4365,13 +4365,14 @@ struct fib6_info *addrconf_f6i_alloc(struct net *net,
 	struct fib6_config cfg = {
 		.fc_table = l3mdev_fib_table(idev->dev) ? : RT6_TABLE_LOCAL,
 		.fc_ifindex = idev->dev->ifindex,
-		.fc_flags = RTF_UP | RTF_ADDRCONF | RTF_NONEXTHOP,
+		.fc_flags = RTF_UP | RTF_NONEXTHOP,
 		.fc_dst = *addr,
 		.fc_dst_len = 128,
 		.fc_protocol = RTPROT_KERNEL,
 		.fc_nlinfo.nl_net = net,
 		.fc_ignore_dev_down = true,
 	};
+	struct fib6_info *f6i;
 
 	if (anycast) {
 		cfg.fc_type = RTN_ANYCAST;
@@ -4381,7 +4382,10 @@ struct fib6_info *addrconf_f6i_alloc(struct net *net,
 		cfg.fc_flags |= RTF_LOCAL;
 	}
 
-	return ip6_route_info_create(&cfg, gfp_flags, NULL);
+	f6i = ip6_route_info_create(&cfg, gfp_flags, NULL);
+	if (f6i)
+		f6i->dst_nocount = true;
+	return f6i;
 }
 
 /* remove deleted ip from prefsrc entries */
-- 
2.23.0.187.g17f5b7556c-goog

Re: [PATCH v2] net-ipv6: fix excessive RTF_ADDRCONF flag on ::1/128 local route (and others)

[David Ahern]

On 9/2/19 10:23 AM, Maciej Żenczykowski wrote:
> From: Maciej Żenczykowski <maze@google.com>
> 
> There is a subtle change in behaviour introduced by:
>   commit c7a1ce397adacaf5d4bb2eab0a738b5f80dc3e43
>   'ipv6: Change addrconf_f6i_alloc to use ip6_route_info_create'
> 
> Before that patch /proc/net/ipv6_route includes:
> 00000000000000000000000000000001 80 00000000000000000000000000000000 00 00000000000000000000000000000000 00000000 00000003 00000000 80200001 lo
> 
> Afterwards /proc/net/ipv6_route includes:
> 00000000000000000000000000000001 80 00000000000000000000000000000000 00 00000000000000000000000000000000 00000000 00000002 00000000 80240001 lo
> 
> ie. the above commit causes the ::1/128 local (automatic) route to be flagged with RTF_ADDRCONF (0x040000).
> 
> AFAICT, this is incorrect since these routes are *not* coming from RA's.
> 
> As such, this patch restores the old behaviour.
> 
> Fixes: c7a1ce397adacaf5d4bb2eab0a738b5f80dc3e43
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Lorenzo Colitti <lorenzo@google.com>
> Signed-off-by: Maciej Żenczykowski <maze@google.com>
> ---
>  net/ipv6/route.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 

Looks correct to me. Thanks for the patch.

Reviewed-by: David Ahern <dsahern@gmail.com>

Re: [PATCH] net-ipv6: fix excessive RTF_ADDRCONF flag on ::1/128 local route (and others)

[David Ahern]

On 9/1/19 8:12 PM, Lorenzo Colitti wrote:
> Not sure if this patch is the right fix, though, because it breaks
> things in the opposite direction: even routes created by an IPv6
> address added by receiving an RA will no longer have RTF_ADDRCONF.
> Perhaps add something like this as well?
> 
>  struct fib6_info *addrconf_f6i_alloc(struct net *net, struct inet6_dev *idev,
> -                                     const struct in6_addr *addr, bool anycast,
> -                                     const struct in6_addr *addr, u8 flags,
>                                       gfp_t gfp_flags);
> 
> flags would be RTF_ANYCAST iff the code previously called with true,
> and RTF_ADDRCONF if called by a function that is adding an IPv6
> address coming from an RA.

addrconf_f6i_alloc is used for addresses added by userspace
(ipv6_add_addr) and anycast. ie., from what I can see it is not used for RAs

Re: [PATCH] net-ipv6: fix excessive RTF_ADDRCONF flag on ::1/128 local route (and others)

[Lorenzo Colitti]

On Tue, Sep 3, 2019 at 11:18 AM David Ahern <dsahern@gmail.com> wrote:
> addrconf_f6i_alloc is used for addresses added by userspace
> (ipv6_add_addr) and anycast. ie., from what I can see it is not used for RAs

Isn't ipv6_add_addr called by addrconf_prefix_rcv_add_addr, which is
called by addrconf_prefix_rcv, which is called by
ndisc_router_discovery? That is what happens when we process an RA;
AFAICS manual configuration is inet6_addr_add, not ipv6_add_addr.

Maciej, with this patch, do SLAAC addresses still have RTF_ADDRCONF?
Per my previous message, my assumption would be no, but I might be
misreading the code.

Re: [PATCH] net-ipv6: fix excessive RTF_ADDRCONF flag on ::1/128 local route (and others)

[Maciej Żenczykowski]

Well, if you look at the commit my commit is fixing, ie.
  commit c7a1ce397adacaf5d4bb2eab0a738b5f80dc3e43
then you'll see this in the commit description:
  "- dst_nocount is handled by the RTF_ADDRCONF flag"
and the patch diff itself is from
  "f6i->fib6_flags = RTF_UP | RTF_NONEXTHOP;
   f6i->dst_nocount = true;"
to
  " .fc_flags = RTF_UP | RTF_ADDRCONF | RTF_NONEXTHOP,"

(and RTF_ANYCAST or RTF_LOCAL is later or'ed in in both versions of the code)

so I'm pretty sure that patch adds ADDRCONF unconditionally to that
function, and my commit unconditionally removes it.

Perhaps since then the call graph has changed???

Unfortunately I'm already in Europe on ancient ipv6 free networks and
since the office move my ipv6 lab is still not up (something about the
newly wired office jacks being blue which means corp, instead of any
other colour for a lab...) so I don't actually have an easy way to
test ipv6 slaac behaviour. :-(

On Tue, Sep 3, 2019 at 6:58 AM Lorenzo Colitti <lorenzo@google.com> wrote:
>
> On Tue, Sep 3, 2019 at 11:18 AM David Ahern <dsahern@gmail.com> wrote:
> > addrconf_f6i_alloc is used for addresses added by userspace
> > (ipv6_add_addr) and anycast. ie., from what I can see it is not used for RAs
>
> Isn't ipv6_add_addr called by addrconf_prefix_rcv_add_addr, which is
> called by addrconf_prefix_rcv, which is called by
> ndisc_router_discovery? That is what happens when we process an RA;
> AFAICS manual configuration is inet6_addr_add, not ipv6_add_addr.
>
> Maciej, with this patch, do SLAAC addresses still have RTF_ADDRCONF?
> Per my previous message, my assumption would be no, but I might be
> misreading the code.

Re: [PATCH] net-ipv6: fix excessive RTF_ADDRCONF flag on ::1/128 local route (and others)

[David Ahern]

On 9/3/19 6:17 AM, Maciej Żenczykowski wrote:
> Well, if you look at the commit my commit is fixing, ie.
>   commit c7a1ce397adacaf5d4bb2eab0a738b5f80dc3e43
> then you'll see this in the commit description:
>   "- dst_nocount is handled by the RTF_ADDRCONF flag"
> and the patch diff itself is from
>   "f6i->fib6_flags = RTF_UP | RTF_NONEXTHOP;
>    f6i->dst_nocount = true;"
> to
>   " .fc_flags = RTF_UP | RTF_ADDRCONF | RTF_NONEXTHOP,"
> 
> (and RTF_ANYCAST or RTF_LOCAL is later or'ed in in both versions of the code)
> 
> so I'm pretty sure that patch adds ADDRCONF unconditionally to that
> function, and my commit unconditionally removes it.
> 

exactly. It was shortsighted of me to add the ADDRCONF flag and removing
it reverts back to the previous behavior.

When I enable radvd, I do see the flag set when it should be and not for
other addresses. I believe the patch is correct.

Re: [PATCH] net-ipv6: fix excessive RTF_ADDRCONF flag on ::1/128 local route (and others)

[Lorenzo Colitti]

On Wed, Sep 4, 2019 at 4:45 AM David Ahern <dsahern@gmail.com> wrote:
>
> exactly. It was shortsighted of me to add the ADDRCONF flag and removing
> it reverts back to the previous behavior.
>
> When I enable radvd, I do see the flag set when it should be and not for
> other addresses. I believe the patch is correct.

Ah, wait, I was confused. Sorry.

What I was saying is that RTF_ADDRCONF flag should be set on the local
table /128 route for the IPv6 addresses configured by RAs. The way
those are configured is ndisc_router_discovery -> addrconf_prefix_rcv
-> addrconf_prefix_rcv_add_addr -> ipv6_add_addr ->
addrconf_f6i_alloc. Because in this patch, addrconf_f6i_alloc
unconditionally clears RTF_ADDRCONF, I didn't see how the flag could
be set. But I now realize that that was not happening before David's
commit, either: in 5.1 those routes were not created with RTF_ADDRCONF
either.

In other words: before 5.1, the /128 routes in the local table to IPv6
addresses created by SLAAC did not have RTF_ADDRCONF. After David's
commit, they did, because *all* /128 routes to IPv6 addresses had
RTF_ADDRCONF (correct for SLAAC adresses, but definitely incorrect for
manual addresses, loopback, etc.). If this commit is applied, we'll go
back to the 5.1 state. In the future it would be good to ensure that
at least the /128 routes created by SLAAC do have RTF_ADDRCONF, but no
need to do so in this commit.

Re: [PATCH v2] net-ipv6: fix excessive RTF_ADDRCONF flag on ::1/128 local route (and others)

[David Miller]

From: Maciej Żenczykowski <zenczykowski@gmail.com>
Date: Mon,  2 Sep 2019 09:23:36 -0700

> From: Maciej Żenczykowski <maze@google.com>
> 
> There is a subtle change in behaviour introduced by:
>   commit c7a1ce397adacaf5d4bb2eab0a738b5f80dc3e43
>   'ipv6: Change addrconf_f6i_alloc to use ip6_route_info_create'
> 
> Before that patch /proc/net/ipv6_route includes:
> 00000000000000000000000000000001 80 00000000000000000000000000000000 00 00000000000000000000000000000000 00000000 00000003 00000000 80200001 lo
> 
> Afterwards /proc/net/ipv6_route includes:
> 00000000000000000000000000000001 80 00000000000000000000000000000000 00 00000000000000000000000000000000 00000000 00000002 00000000 80240001 lo
> 
> ie. the above commit causes the ::1/128 local (automatic) route to be flagged with RTF_ADDRCONF (0x040000).
> 
> AFAICT, this is incorrect since these routes are *not* coming from RA's.
> 
> As such, this patch restores the old behaviour.
> 
> Fixes: c7a1ce397adacaf5d4bb2eab0a738b5f80dc3e43

Please format your Fixes: tag properly next time, I fixed it up for you.

> Cc: David Ahern <dsahern@gmail.com>
> Cc: Lorenzo Colitti <lorenzo@google.com>
> Signed-off-by: Maciej Żenczykowski <maze@google.com>

Applied and queued up for -stable.

Re: [PATCH v2] net-ipv6: fix excessive RTF_ADDRCONF flag on ::1/128 local route (and others)

[Eric Dumazet]

On 9/2/19 6:23 PM, Maciej Żenczykowski wrote:
> From: Maciej Żenczykowski <maze@google.com>
> 
> There is a subtle change in behaviour introduced by:
>   commit c7a1ce397adacaf5d4bb2eab0a738b5f80dc3e43
>   'ipv6: Change addrconf_f6i_alloc to use ip6_route_info_create'
> 
> Before that patch /proc/net/ipv6_route includes:
> 00000000000000000000000000000001 80 00000000000000000000000000000000 00 00000000000000000000000000000000 00000000 00000003 00000000 80200001 lo
> 
> Afterwards /proc/net/ipv6_route includes:
> 00000000000000000000000000000001 80 00000000000000000000000000000000 00 00000000000000000000000000000000 00000000 00000002 00000000 80240001 lo
> 
> ie. the above commit causes the ::1/128 local (automatic) route to be flagged with RTF_ADDRCONF (0x040000).
> 
> AFAICT, this is incorrect since these routes are *not* coming from RA's.
> 
> As such, this patch restores the old behaviour.
> 
> Fixes: c7a1ce397adacaf5d4bb2eab0a738b5f80dc3e43
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Lorenzo Colitti <lorenzo@google.com>
> Signed-off-by: Maciej Żenczykowski <maze@google.com>
> ---
>  net/ipv6/route.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 558c6c68855f..516b2e568dae 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -4365,13 +4365,14 @@ struct fib6_info *addrconf_f6i_alloc(struct net *net,
>  	struct fib6_config cfg = {
>  		.fc_table = l3mdev_fib_table(idev->dev) ? : RT6_TABLE_LOCAL,
>  		.fc_ifindex = idev->dev->ifindex,
> -		.fc_flags = RTF_UP | RTF_ADDRCONF | RTF_NONEXTHOP,
> +		.fc_flags = RTF_UP | RTF_NONEXTHOP,
>  		.fc_dst = *addr,
>  		.fc_dst_len = 128,
>  		.fc_protocol = RTPROT_KERNEL,
>  		.fc_nlinfo.nl_net = net,
>  		.fc_ignore_dev_down = true,
>  	};
> +	struct fib6_info *f6i;
>  
>  	if (anycast) {
>  		cfg.fc_type = RTN_ANYCAST;
> @@ -4381,7 +4382,10 @@ struct fib6_info *addrconf_f6i_alloc(struct net *net,
>  		cfg.fc_flags |= RTF_LOCAL;
>  	}
>  
> -	return ip6_route_info_create(&cfg, gfp_flags, NULL);
> +	f6i = ip6_route_info_create(&cfg, gfp_flags, NULL);
> +	if (f6i)
> +		f6i->dst_nocount = true;

Shouldn't it use 

	if (!IS_ERR(f6i))
		f6i->dst_nocount = true;

???


> +	return f6i;
>  }
>  
>  /* remove deleted ip from prefsrc entries */
> 

Re: [PATCH v2] net-ipv6: fix excessive RTF_ADDRCONF flag on ::1/128 local route (and others)

[Maciej Żenczykowski]

> Shouldn't it use
>
>         if (!IS_ERR(f6i))
>                 f6i->dst_nocount = true;
>
> ???

Yes, certainly.  I'll send a fix.

[PATCH] net-ipv6: addrconf_f6i_alloc - fix non-null pointer check to !IS_ERR()

[Maciej Żenczykowski]

From: Maciej Żenczykowski <maze@google.com>

Fixes a stupid bug I recently introduced...
ip6_route_info_create() returns an ERR_PTR(err) and not a NULL on error.

Fixes: d55a2e374a94 ("net-ipv6: fix excessive RTF_ADDRCONF flag on ::1/128 local route (and others)'")
Cc: David Ahern <dsahern@gmail.com>
Cc: Lorenzo Colitti <lorenzo@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 net/ipv6/route.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 003562dd3395..2fb2b913214c 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -4383,7 +4383,7 @@ struct fib6_info *addrconf_f6i_alloc(struct net *net,
 	}
 
 	f6i = ip6_route_info_create(&cfg, gfp_flags, NULL);
-	if (f6i)
+	if (!IS_ERR(f6i))
 		f6i->dst_nocount = true;
 	return f6i;
 }
-- 
2.23.0.187.g17f5b7556c-goog

Re: [PATCH] net-ipv6: addrconf_f6i_alloc - fix non-null pointer check to !IS_ERR()

[Eric Dumazet]

On Fri, Sep 6, 2019 at 5:56 AM Maciej Żenczykowski
<zenczykowski@gmail.com> wrote:
>
> From: Maciej Żenczykowski <maze@google.com>
>
> Fixes a stupid bug I recently introduced...
> ip6_route_info_create() returns an ERR_PTR(err) and not a NULL on error.
>
> Fixes: d55a2e374a94 ("net-ipv6: fix excessive RTF_ADDRCONF flag on ::1/128 local route (and others)'")
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Lorenzo Colitti <lorenzo@google.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Maciej Żenczykowski <maze@google.com>
> ---

Reported-by: syzbot <syzkaller@googlegroups.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>

Thanks.

Re: [PATCH] net-ipv6: addrconf_f6i_alloc - fix non-null pointer check to !IS_ERR()

[David Miller]

From: Maciej Żenczykowski <zenczykowski@gmail.com>
Date: Thu,  5 Sep 2019 20:56:37 -0700

> From: Maciej Żenczykowski <maze@google.com>
> 
> Fixes a stupid bug I recently introduced...
> ip6_route_info_create() returns an ERR_PTR(err) and not a NULL on error.
> 
> Fixes: d55a2e374a94 ("net-ipv6: fix excessive RTF_ADDRCONF flag on ::1/128 local route (and others)'")
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Lorenzo Colitti <lorenzo@google.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Maciej Żenczykowski <maze@google.com>

Applied and queued up for -stable since I queued up the patch this fixes.