netdev - Re: [patch net-next v3 01/11] net: devlink: make sure that devlink_try_get() works with valid pointer during xarray iteration

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:   Wed, 20 Jul 2022 23:22:58 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, idosch@...dia.com,
        petrm@...dia.com, pabeni@...hat.com, edumazet@...gle.com,
        mlxsw@...dia.com, saeedm@...dia.com, snelson@...sando.io
Subject: Re: [patch net-next v3 01/11] net: devlink: make sure that
 devlink_try_get() works with valid pointer during xarray iteration

On Thu, 21 Jul 2022 07:51:37 +0200 Jiri Pirko wrote:
> >Hm. I always assumed we'd just use the xa_lock(). Unmarking the
> >instance as registered takes that lock which provides a natural 
> >barrier for others trying to take a reference.  
> 
> I guess that the xa_lock() scheme could work, as far as I see it. But
> what's wrong with the rcu scheme? I actually find it quite neat. No need
> to have another odd iteration helpers. We just benefit of xa_array rcu
> internals to make sure devlink pointer is valid at the time we make a
> reference. Very clear.

Nothing strongly against the RCU scheme, TBH. Just didn't expect it.
I can concoct some argument like it's one extra sync primitive we
haven't had to think about in devlink so far, but really if you prefer
RCU, I don't mind.

I do like the idea of wrapping the iteration into our own helper, tho.
Contains the implementation details of the iteration nicely. I didn't
look in sufficient detail but I would have even considered rolling the
namespace check into it for dump.