linux-kernel - Re: [RFC PATCH] LKMM: Add ctrl_dep() macro for control dependency

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, 29 Sep 2021 15:27:06 -0400 (EDT)
From:   Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Will Deacon <will@...nel.org>, paulmck <paulmck@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Segher Boessenkool <segher@...nel.crashing.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Alan Stern <stern@...land.harvard.edu>,
        Andrea Parri <parri.andrea@...il.com>,
        Boqun Feng <boqun.feng@...il.com>,
        Nicholas Piggin <npiggin@...il.com>,
        David Howells <dhowells@...hat.com>,
        j alglave <j.alglave@....ac.uk>,
        luc maranget <luc.maranget@...ia.fr>,
        akiyks <akiyks@...il.com>,
        linux-toolchains <linux-toolchains@...r.kernel.org>,
        linux-arch <linux-arch@...r.kernel.org>
Subject: Re: [RFC PATCH] LKMM: Add ctrl_dep() macro for control dependency

----- On Sep 29, 2021, at 10:47 AM, Linus Torvalds torvalds@...ux-foundation.org wrote:

> On Tue, Sep 28, 2021 at 2:15 PM Mathieu Desnoyers
> <mathieu.desnoyers@...icios.com> wrote:
>>
>> Introduce the ctrl_dep macro in the generic headers, and use it
>> everywhere it appears relevant.
> 
> The control dependency is so subtle - just see our discussions - that
> I really think every single use of it needs to have a comment about
> why it's needed.

I agree with you on thorough documentation of each control dependency,
perhaps just not about documentation of all compiler optimizations
affecting each of them.

> 
> Right now, that patch seems to just sprinkle them more or less
> randomly. That's absolutely not what I want. It will just mean that
> other people start sprinkling them randomly even more, and nobody will
> dare remove them.

Note that I have not found that many uses of control dependencies in the
kernel tree. When they are used, this happens to be code where speed
really matters though.

> So I'd literally want a comment about "this needs a control
> dependency, because otherwise the compiler could merge the two
> identical stores X and Y".

My hope with this ctrl_dep() macro is to remove at least some of
the caveats to keep in mind when using control dependency ordering.
Requiring to keep track of all relevant compiler optimizations on all
architectures while reasoning about memory barriers is error-prone.

> When you have a READ_ONCE() in the conditional, and a WRITE_ONCE() in
> the statement protected by the conditional, there is *no* need to
> randomly sprinkle noise that doesn't matter.

The main advantage in doing so would be documentation, both in terms of
letting the compiler know that this control dependency matters for
ordering, and in terms of simplifying the set of caveats to document
in Documentation/memory-barriers.txt.

> And if there *is* need ("look, we have that same store in both the if-
> and the else-statement" or whatever), then say so, and state that
> thing.

If we go for only using ctrl_dep() for scenarios which require it for
documented reasons, then we would need to leave in place all the
caveats details in Documentation/memory-barriers.txt, and document
that in those scenarios ctrl_dep() should be used. This would be a
starting point I guess.

Thanks,

Mathieu

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