linux-kernel - Re: [PATCH v4] Fixing directly deferencing a _

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:   Sun, 12 Nov 2023 14:56:05 -0500
From:   Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To:     Abhinav Singh <singhabhinav9051571833@...il.com>,
        akpm@...ux-foundation.org, brauner@...nel.org, surenb@...gle.com,
        mst@...hat.com, michael.christie@...cle.com, mjguzik@...il.com,
        npiggin@...il.com, peterz@...radead.org,
        paulmck <paulmck@...nel.org>
Cc:     linux-kernel@...r.kernel.org,
        linux-kernel-mentees@...ts.linuxfoundation.org
Subject: Re: [PATCH v4] Fixing directly deferencing a __rcu pointer warning

On 2023-11-12 14:30, Abhinav Singh wrote:
> This patch fixes the sparse warning with this message
> "dereference of noderef expression" , in this context
> it means about directly dereferencing a pointer tagged
> with __rcu annotation.
> 
> Dereferencing the pointers tagged with __rcu directly should
> always be avoided according to the docs. There is a rcu helper
> function rcu_dereference(...) to use when dereferencing a __rcu
> pointer inside rcu read side critical sections. This function
> returns the non __rcu tagged pointer which can be dereferenced
> just like a normal pointer.
> 
> I tested with `lockdep` enabled, with these config options
> `CONFIG_PROVE_RCU` and `CONFIG_PROVE_LOCKING` enabled and it
> booted just fine. To confirm if lockdep was really enabled
> I found these paths inside the qemu virtual envirnoment.

I did not see actions taken nor answer on those comments:

https://lore.kernel.org/all/CAGudoHEfjSAim6Hh-qYPY+qi8nbLx7J3YdpGgFwSvD7xbeYR3w@mail.gmail.com/

Basically, the missing annotation here can be either:

- A missing rcu_dereference, if the intent is to use the pointer while protecting
   with with a read-side critical section,
- A missing rcu_dereference_protected(), if the use of the pointer is protected by
   a lock.

I don't really care if rcu_dereference happens to work in testing or not. _If_ the
intended design is that this rcu pointer is protected by a lock, or being used before
becoming published elsewhere, then using rcu_dereference to silence the warning is
wrong. (note: I did not do a review of the affected code, but I would expect the
commit message to take care of going through this level of detail)

And the fact that this aspect of the feedback has been hidden under the rug worries
me.

Thanks,

Mathieu


-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com