netdev - Re: [patch net-next 1/7] ipv4: fib: Only flush FIB aliases belonging to currently flushed table

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 for Android: free password hash cracker in your pocket

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

Message-ID: <1486666849.7829.67.camel@intel.com>
Date:   Thu, 9 Feb 2017 19:00:51 +0000
From:   "Duyck, Alexander H" <alexander.h.duyck@...el.com>
To:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "jiri@...nulli.us" <jiri@...nulli.us>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        "idosch@...lanox.com" <idosch@...lanox.com>,
        "eladr@...lanox.com" <eladr@...lanox.com>,
        "mlxsw@...lanox.com" <mlxsw@...lanox.com>,
        "kaber@...sh.net" <kaber@...sh.net>
Subject: Re: [patch net-next 1/7] ipv4: fib: Only flush FIB aliases
 belonging to currently flushed table

On Thu, 2017-02-09 at 10:28 +0100, Jiri Pirko wrote:
> From: Ido Schimmel <idosch@...lanox.com>
> 
> In case the MAIN table is flushed and its trie is shared with the LOCAL
> table, then we might be flushing FIB aliases belonging to the latter.
> This can lead to FIB_ENTRY_DEL notifications sent with the wrong table
> ID.
> 
> The above doesn't affect current listeners, as the table ID is ignored
> during entry deletion, but this will change later in the patchset.
> 
> When flushing a particular table, skip any aliases belonging to a
> different one.
> 
> Signed-off-by: Ido Schimmel <idosch@...lanox.com>
> Signed-off-by: Jiri Pirko <jiri@...lanox.com>
> CC: Alexander Duyck <alexander.h.duyck@...el.com>
> CC: Patrick McHardy <kaber@...sh.net>
> ---
>  net/ipv4/fib_trie.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
> index 2919d1a..5ef4596 100644
> --- a/net/ipv4/fib_trie.c
> +++ b/net/ipv4/fib_trie.c
> @@ -1963,7 +1963,8 @@ int fib_table_flush(struct net *net, struct fib_table *tb)
>  		hlist_for_each_entry_safe(fa, tmp, &n->leaf, fa_list) {
>  			struct fib_info *fi = fa->fa_info;
>  
> -			if (!fi || !(fi->fib_flags & RTNH_F_DEAD)) {
> +			if (!fi || !(fi->fib_flags & RTNH_F_DEAD) ||
> +			    tb->tb_id != fa->tb_id) {
>  				slen = fa->fa_slen;
>  				continue;
>  			}

One change I might make if you end up having to do a v2 would be to
test for the table ID first.  It can end up saving a few cycles in the
whole flushing process since the table ID is in the fib alias instead
of having to dereference the fib info.

That being said, I am just being a bit nit-picky so the code itself is
functionally correct and there is nothing here that should cause any
issues.

Reviewed-by: Alexander Duyck <alexander.h.duyck@...el.com>