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

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

Message-ID: <YtlAtO7o4ksMOF1F@nanopsycho>
Date:   Thu, 21 Jul 2022 14:04:04 +0200
From:   Jiri Pirko <jiri@...dia.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     Jiri Pirko <jiri@...nulli.us>, 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

Thu, Jul 21, 2022 at 08:22:58AM CEST, kuba@...nel.org wrote:
>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.

Hmm, okay. I will think about helpers to contain the
iteration/rcu/refget stuff.

Thanks!