linux-kernel - Re: [PATCH] x86-32: fix cmpxchg8b

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-=whp_j=txQ+bGQqsf5_2fY0vM3U4Ssy6S+Yhg4ugYHdMZQ@mail.gmail.com>
Date: Wed, 26 Jun 2024 08:11:04 -0700
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Uros Bizjak <ubizjak@...il.com>
Cc: Ingo Molnar <mingo@...nel.org>, Borislav Petkov <bp@...en8.de>, 
	Peter Zijlstra <peterz@...radead.org>, linux-kernel@...r.kernel.org, x86@...nel.org, 
	kernel test robot <lkp@...el.com>
Subject: Re: [PATCH] x86-32: fix cmpxchg8b_emu build error with clang

On Wed, 26 Jun 2024 at 00:39, Uros Bizjak <ubizjak@...il.com> wrote:
>
> >
> > Still doing the 'oldp' writeback unconmditionally.  The code generation
> > for the case I checked were the same for both clang and gcc, but until
> > Uros hits me with the big clue-hammer, I think it's the simpler code
> > that leaves room for potentially better optimizations too.
>
> You probably want to look at 44fe84459faf1 ("locking/atomic: Fix
> atomic_try_cmpxchg() semantics") [1] and the long LKML discussion at
> [2].

Christ. That use should be invalid.

The only _atomic_ pointer is "_ptr", not "old". Anybody who gives
something that can change during the operation in "old" is basically
already doing random things.

> --quote--
> This code is broken with the current implementation, the problem is
> with unconditional update of *__po.

I think the only thing broken is that quote, and the crazy expectation
that "old" can change.

But obviously, I had completely forgotten that whole discussion from
seven years ago.

I don't actually find a single use of that invalid code sequence where
somebody would pass a non-private pointer as "oldp". So I really think
that part of the whole discussion was bogus to begin with, and
presumably from some other code base.

IOW, I think that example of a "classical lock-free stack push" is just broken.

That said, I can't find a case where it would matter for code
generation (every use will always do a conditional branch based on the
result, so the conditional assignment is practically speaking always
"static" anyway by the time you do branch following.

So I'll just send out a minimal patch with *only* the %esi changes.

           Linus