netdev - Re: [patch net-next 11/17] ipv6: fib: Allow non-FIB users to take reference on route

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]

Message-ID: <ad7666d8-ede5-9518-e169-3f5f6f0b6798@gmail.com>
Date:   Wed, 19 Jul 2017 10:29:14 -0600
From:   David Ahern <dsahern@...il.com>
To:     Ido Schimmel <idosch@...lanox.com>
Cc:     Jiri Pirko <jiri@...nulli.us>, netdev@...r.kernel.org,
        davem@...emloft.net, mlxsw@...lanox.com, roopa@...ulusnetworks.com,
        nikolay@...ulusnetworks.com, kafai@...com,
        hannes@...essinduktion.org, yoshfuji@...ux-ipv6.org,
        edumazet@...gle.com, yanhaishuang@...s.chinamobile.com
Subject: Re: [patch net-next 11/17] ipv6: fib: Allow non-FIB users to take
 reference on route

On 7/19/17 10:17 AM, Ido Schimmel wrote:
> I did exactly that in the beginning, but it didn't sit right with me for
> the exact reason you mentioned - it can be a PITA to debug.
> 
> If we use rt6i_ref for something other than FIB references, then it
> breaks existing code that relies on rt6i_ref being 0 to indicate it's
> no longer used by the FIB. A non-zero value can now mean "not used by
> the FIB, but waiting for some module to drop the reference in its
> workqueue".
> 
> The BUG_ON() mentioned in the commit message is just one example.
> Another check was added by you in commit 8048ced9b.
> 
> So I think we both want the same thing, but I'm not sure how your
> approach is safer.

A single reference counter rt6i_ref is best.

There are 2 reads of that counter to determine if the rt is still in the
FIB. Both of those stem from side effects of using the 'lo' for the
device for host addresses. I think an explicit flag can be used for that
purpose instead of trying to deduce it from the reference counter. The
commit you referenced copied what is done in init_loopback for
consistency (both have same end goal).