linux-kernel - Re: [GIT PULL] sigqueue cache fix

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]

Message-ID: <CAHk-=wjrd6-ZHyQGznpM+O0CtTHjzZ5P2Ozddh68WmDH9c+hBg@mail.gmail.com>
Date:   Mon, 28 Jun 2021 12:02:56 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Ingo Molnar <mingo@...nel.org>
Cc:     Christian Brauner <christian.brauner@...ntu.com>,
        Oleg Nesterov <oleg@...hat.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [GIT PULL] sigqueue cache fix

On Mon, Jun 28, 2021 at 11:47 AM Ingo Molnar <mingo@...nel.org> wrote:
>
> But even if release_posix_timer() is changed to call sigqueue_free() with
> IRQs disabled, or sigqueue_free() disables interrupts itself, I think we
> need to be mindful of the Consumer <-> Producer SMP races, which only
> appear to be safe due to an accidental barrier by free_uid().

Oh, I agree. The SMP memory ordering issues are subtle and while I
suspect they aren't something that can be triggered in reality, it's
an example of how broken some WRITE_ONCE -> READ_ONCE serialization
is.

I don't mind READ_ONCE/WRITE_ONCE in general, but I absolutely detest
them when they are used to hide KCSAN things, and when they are used
randomly for pointers.

I think they are much better suited for one-time flag things, and
pretty much every time you see a READ_ONCE/WRITE_ONCE you should
likely see a barrier somewhere.

And no, we don't want them to be some subtle barriers that are just
"in the context of this code, we can depend on the barrier in that
other function".

In general, if you have a WRITE_ONCE -> READ_ONCE chain an dnot some
obvious required barrier for other reasons right *there*, it should
almost certainly not be one of those ONCE things at all. It should be
a proper smp_store_release() -> smp_load_acquire(). Those have real -
and on sane hardware cheap - memory orderings.

And as the whole - and only - point of the sigqueue cache was a very
subtle performance and latency issue, I don't think we want to use
locks or atomics. It's why my revert commit suggests re-purposing the
"percpu_cmpxchg()" functionality: that would likely be a good model at
least for x86 and arm.

But while we have "percpu_cmpxchg()", I don't think we currently don't
really have that kind of operation where it

 (a) works on a non-percpu variable (but we can force casts, I guess)

 (b) has "acquire" semantics

We do have the *atomic* cmpxchg with acquire semantics, but atomics
are rather more expensive than we'd probably really want.

                 Linus