netdev - Re: [PATCH v2 net] ipv6: Use global sernum for dst validation with nexthop objects

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] [day] [month] [year] [list]

Message-Id: <20200501.124713.1763767482562707777.davem@davemloft.net>
Date:   Fri, 01 May 2020 12:47:13 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     dsahern@...nel.org
Cc:     kuba@...nel.org, netdev@...r.kernel.org, eric.dumazet@...il.com,
        nikolay@...ulusnetworks.com
Subject: Re: [PATCH v2 net] ipv6: Use global sernum for dst validation with
 nexthop objects

From: David Ahern <dsahern@...nel.org>
Date: Fri,  1 May 2020 08:53:08 -0600

> Nik reported a bug with pcpu dst cache when nexthop objects are
> used illustrated by the following:
 ...
> Conversion of FIB entries to work with external nexthop objects
> missed an important difference between IPv4 and IPv6 - how dst
> entries are invalidated when the FIB changes. IPv4 has a per-network
> namespace generation id (rt_genid) that is bumped on changes to the FIB.
> Checking if a dst_entry is still valid means comparing rt_genid in the
> rtable to the current value of rt_genid for the namespace.
> 
> IPv6 also has a per network namespace counter, fib6_sernum, but the
> count is saved per fib6_node. With the per-node counter only dst_entries
> based on fib entries under the node are invalidated when changes are
> made to the routes - limiting the scope of invalidations. IPv6 uses a
> reference in the rt6_info, 'from', to track the corresponding fib entry
> used to create the dst_entry. When validating a dst_entry, the 'from'
> is used to backtrack to the fib6_node and check the sernum of it to the
> cookie passed to the dst_check operation.
> 
> With the inline format (nexthop definition inline with the fib6_info),
> dst_entries cached in the fib6_nh have a 1:1 correlation between fib
> entries, nexthop data and dst_entries. With external nexthops, IPv6
> looks more like IPv4 which means multiple fib entries across disparate
> fib6_nodes can all reference the same fib6_nh. That means validation
> of dst_entries based on external nexthops needs to use the IPv4 format
> - the per-network namespace counter.
> 
> Add sernum to rt6_info and set it when creating a pcpu dst entry. Update
> rt6_get_cookie to return sernum if it is set and update dst_check for
> IPv6 to look for sernum set and based the check on it if so. Finally,
> rt6_get_pcpu_route needs to validate the cached entry before returning
> a pcpu entry (similar to the rt_cache_valid calls in __mkroute_input and
> __mkroute_output for IPv4).
> 
> This problem only affects routes using the new, external nexthops.
> 
> Thanks to the kbuild test robot for catching the IS_ENABLED needed
> around rt_genid_ipv6 before I sent this out.
> 
> Fixes: 5b98324ebe29 ("ipv6: Allow routes to use nexthop objects")
> Reported-by: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
> Signed-off-by: David Ahern <dsahern@...nel.org>
> Reviewed-by: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
> Tested-by: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>

Applied and queued up for -stable, thanks David.