netdev - Re: [PATCH net-next v7 08/11] ipv6: Dump route exceptions if requested

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 21 Jun 2019 16:21:56 -0600
From:   David Ahern <dsahern@...il.com>
To:     Stefano Brivio <sbrivio@...hat.com>,
        David Miller <davem@...emloft.net>
Cc:     Jianlin Shi <jishi@...hat.com>, Wei Wang <weiwan@...gle.com>,
        Martin KaFai Lau <kafai@...com>,
        Eric Dumazet <edumazet@...gle.com>,
        Matti Vaittinen <matti.vaittinen@...rohmeurope.com>,
        netdev@...r.kernel.org
Subject: Re: [PATCH net-next v7 08/11] ipv6: Dump route exceptions if
 requested

On 6/21/19 9:45 AM, Stefano Brivio wrote:
> Since commit 2b760fcf5cfb ("ipv6: hook up exception table to store dst
> cache"), route exceptions reside in a separate hash table, and won't be
> found by walking the FIB, so they won't be dumped to userspace on a
> RTM_GETROUTE message.
> 
> This causes 'ip -6 route list cache' and 'ip -6 route flush cache' to
> have no function anymore:
> 
>  # ip -6 route get fc00:3::1
>  fc00:3::1 via fc00:1::2 dev veth_A-R1 src fc00:1::1 metric 1024 expires 539sec mtu 1400 pref medium
>  # ip -6 route get fc00:4::1
>  fc00:4::1 via fc00:2::2 dev veth_A-R2 src fc00:2::1 metric 1024 expires 536sec mtu 1500 pref medium
>  # ip -6 route list cache
>  # ip -6 route flush cache
>  # ip -6 route get fc00:3::1
>  fc00:3::1 via fc00:1::2 dev veth_A-R1 src fc00:1::1 metric 1024 expires 520sec mtu 1400 pref medium
>  # ip -6 route get fc00:4::1
>  fc00:4::1 via fc00:2::2 dev veth_A-R2 src fc00:2::1 metric 1024 expires 519sec mtu 1500 pref medium
> 
> because iproute2 lists cached routes using RTM_GETROUTE, and flushes them
> by listing all the routes, and deleting them with RTM_DELROUTE one by one.
> 
> If cached routes are requested using the RTM_F_CLONED flag together with
> strict checking, or if no strict checking is requested (and hence we can't
> consistently apply filters), look up exceptions in the hash table
> associated with the current fib6_info in rt6_dump_route(), and, if present
> and not expired, add them to the dump.
> 
> We might be unable to dump all the entries for a given node in a single
> message, so keep track of how many entries were handled for the current
> node in fib6_walker, and skip that amount in case we start from the same
> partially dumped node.
> 
> When a partial dump restarts, as the starting node might change when
> 'sernum' changes, we have no guarantee that we need to skip the same
> amount of in-node entries. Therefore, we need two counters, and we need to
> zero the in-node counter if the node from which the dump is resumed
> differs.
> 
> Note that, with the current version of iproute2, this only fixes the
> 'ip -6 route list cache': on a flush command, iproute2 doesn't pass
> RTM_F_CLONED and, due to this inconsistency, 'ip -6 route flush cache' is
> still unable to fetch the routes to be flushed. This will be addressed in
> a patch for iproute2.
> 
> To flush cached routes, a procfs entry could be introduced instead: that's
> how it works for IPv4. We already have a rt6_flush_exception() function
> ready to be wired to it. However, this would not solve the issue for
> listing.
> 

...

> 
> Reported-by: Jianlin Shi <jishi@...hat.com>
> Fixes: 2b760fcf5cfb ("ipv6: hook up exception table to store dst cache")
> Signed-off-by: Stefano Brivio <sbrivio@...hat.com>
> ---
>  include/net/ip6_fib.h   |   1 +
>  include/net/ip6_route.h |   2 +-
>  net/ipv6/ip6_fib.c      |  12 ++++-
>  net/ipv6/route.c        | 114 ++++++++++++++++++++++++++++++++++++----
>  4 files changed, 116 insertions(+), 13 deletions(-)
> 

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