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:   Tue, 7 Mar 2023 12:54:07 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Jakub Jelinek <jakub@...hat.com>
Cc:     Segher Boessenkool <segher@...nel.crashing.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Stephane Eranian <eranian@...gle.com>,
        linux-kernel@...r.kernel.org, tony.luck@...el.com,
        reinette.chatre@...el.com, fenghua.yu@...el.com,
        peternewman@...gle.com, james.morse@....com, babu.moger@....com,
        ananth.narayan@....com, vschneid@...hat.com,
        Nathan Chancellor <nathan@...nel.org>,
        clang-built-linux <llvm@...ts.linux.dev>,
        Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>, linux-toolchains@...r.kernel.org
Subject: Re: [PATCH] x86/resctrl: avoid compiler optimization in __resctrl_sched_in

On Tue, Mar 7, 2023 at 12:43 PM Jakub Jelinek <jakub@...hat.com> wrote:
>
> Are we actually talking here about "p" constraint or about p/P (x86) modifiers
> (asm ("%p0" : : "i" (42));)?

In this case it's actually the "p" constraint.

And the "percpu_stable_op()" thing uses it exactly because it thinks
it wants the "I don't care about data dependencies in memory, because
this memory location doesn't change".

Of course, that "this memory location doesn't change" is then always
technically a lie. It's exactly things like "current" that obviously
*does* change in the big picture, but from the context of a particular
_thread_, "current" is a fixed value.

Which then causes problems when you use that "percpu_stable_op()"
thing from within the scheduler (generally without being *aware* of
this issue at all, since the use is hidden a few macro expansions
down).

I think the problem is that the <asm/resctrl.h> code is disgusting and
horrible in multiple ways:

 (a) it shouldn't define and declare a static function in a header file

 (b) the resctrl_sched_in() inline function is midesigned to begin with

basically, the actual scheduling functions should *never* look at
"current" at all. They are - byu definition - changing it, and they
already *know* what both the "incoming current" (aka "prev_p") and
"new current" (aka "next_p") are.

So any scheduling function that uses "current" is hot garbage.

In this case, that hot garbage is resctrl_sched_in().

I do not believe this is a compiler issue. This is literally just a
kernel bug, and that resctrl code being very very wrong.

              Linus

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ