netdev - Re: [PATCH net-next 2/8] rtnetlink: add rtnl_register

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]

Date:   Mon, 13 Nov 2017 08:59:49 +0100
From:   Florian Westphal <fw@...len.de>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Florian Westphal <fw@...len.de>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next 2/8] rtnetlink: add rtnl_register_module

Peter Zijlstra <peterz@...radead.org> wrote:
> On Mon, Nov 13, 2017 at 08:21:59AM +0100, Florian Westphal wrote:
> > Reason is that some places do this:
> > 
> > rtnl_register(pf, RTM_FOO, doit, NULL, 0);
> > rtnl_register(pf, RTM_FOO, NULL, dumpit, 0);
> 
> Sure, however,
> 
> > (from different call sites in the stack).
> > > -	if (doit)
> > > -		tab[msgindex].doit = doit;
> > > -	if (dumpit)
> > > -		tab[msgindex].dumpit = dumpit;
> > 
> > Which is the reason for these if () tests.
> 
> then we assign NULL, which is fine, no?

I meant that
1) rtnl_register(pf, RTM_FOO, doit, NULL, 0);
2) rtnl_register(pf, RTM_FOO, NULL, dumpit, 0);

2) overwrites doit() back to NULL.

(it doesn't at the moment due to if() guard quoted above).

We could not do this split, and keep the if () around.

But then we change a member of the link array after it has
been published via rcu_assign_pointer.

AFAIU this is exactly what you want to avoid with this patch.