netdev - Re: [PATCH net-next] net/tls: Use RCU API to access tls

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:   Thu, 4 Aug 2022 09:18:04 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Maxim Mikityanskiy <maximmi@...dia.com>
Cc:     "paulmck@...nel.org" <paulmck@...nel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        Tariq Toukan <tariqt@...dia.com>,
        Gal Pressman <gal@...dia.com>,
        "john.fastabend@...il.com" <john.fastabend@...il.com>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        Saeed Mahameed <saeedm@...dia.com>,
        Boris Pismenny <borisp@...dia.com>
Subject: Re: [PATCH net-next] net/tls: Use RCU API to access tls_ctx->netdev

On Thu, 4 Aug 2022 08:08:37 +0000 Maxim Mikityanskiy wrote:
> 2. ctx->refcount goes down to 0, no one can access ctx->netdev anymore,
> we tear down ctx and need to check whether ctx->netdev is NULL.
> 
>  if (!refcount_dec_and_test(&ctx->refcount))
>          return;
>  netdev = rcu_dereference_protected(ctx->netdev,
>                                     !refcount_read(&ctx->refcount));
>  if (netdev)
>          queue_work(...);
> 
> It's somewhat similar to the "structure is beyond being updated" case,
> but it's ensured by refcount, not by RCU (i.e. you example assigned
> my_rcu_pointer = NULL and called synchronize_rcu() to ensure no one
> touches it, and I ensure that we are the only user of ctx by dropping
> refcount to zero).
> 
> So, hoping that my understanding of your explanation is correct, both
> cases can use any of rcu_access_pointer or rcu_dereference_protected.
> Is there some rule of thumb which one to pick in such case?

IMHO, rcu_dereference_protected() documents why it's safe to
dereference the pointer outside of the rcu read section. 

We are only documenting the writer side locking. The fact that there
is a RCU pointer involved is coincidental - I think we could 
as well be checking the TLS_RX_DEV_DEGRADED bit here.

We can add asserts or comments to document the writer side locking.
Piggy backing on RCU seems coincidental. But again, I'm fine either 
way. I'm just saying this because I really doubt there is a rule of
thumb for this level of detail. It's most likely your call.