linux-kernel - Re: [PATCH v4 02/10] uaccess: Add speculation barrier to copy_from_user

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: <af101dba-b02f-4863-a893-789fa08124d0@kernel.org>
Date: Mon, 17 Nov 2025 08:06:26 +0100
From: "Christophe Leroy (CS GROUP)" <chleroy@...nel.org>
To: Thomas Gleixner <tglx@...utronix.de>,
 Alexander Viro <viro@...iv.linux.org.uk>,
 Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
 Ingo Molnar <mingo@...hat.com>, Peter Zijlstra <peterz@...radead.org>,
 Darren Hart <dvhart@...radead.org>, Davidlohr Bueso <dave@...olabs.net>,
 Andre Almeida <andrealmeid@...lia.com>,
 Andrew Morton <akpm@...ux-foundation.org>, Eric Dumazet
 <edumazet@...gle.com>, Kuniyuki Iwashima <kuniyu@...gle.com>,
 Paolo Abeni <pabeni@...hat.com>, Willem de Bruijn <willemb@...gle.com>,
 "David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>,
 Simon Horman <horms@...nel.org>, Daniel Borkmann <daniel@...earbox.net>,
 Dave Hansen <dave.hansen@...ux.intel.com>,
 Madhavan Srinivasan <maddy@...ux.ibm.com>,
 Michael Ellerman <mpe@...erman.id.au>, Nicholas Piggin <npiggin@...il.com>
Cc: linux-block@...r.kernel.org, linux-fsdevel@...r.kernel.org,
 linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
 linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH v4 02/10] uaccess: Add speculation barrier to
 copy_from_user_iter()

Le 15/11/2025 à 16:51, Thomas Gleixner a écrit :
> On Thu, Nov 06 2025 at 12:31, Christophe Leroy wrote:
>> The results of "access_ok()" can be mis-speculated.  The result is that
>> you can end speculatively:
>>
>> 	if (access_ok(from, size))
>> 		// Right here
> 
> This is actually the wrong patch ordering as the barrier is missing in
> the current code. So please add the missing barrier first.

Patch 1 is there because Linus was worried with the performance 
degradation brought by the barrier on x86, see [1]

[1] 
https://lore.kernel.org/all/CAHk-=wj4P6p1kBVW7aJbWAOGJZkB7fXFmwaXLieBRhjmvnWgvQ@mail.gmail.com/

If we change the order, it means we first degrade performance on x86 
with patch 1 then we fix that degradation with patch 2. It seems more 
natural to first ensure that the barrier won't degrade x86 then add the 
barrier.

An alternative is to squash both patches together, after all they touch 
the exact same part of the code.

Let me know what you prefer:
1/ Leave in that order to avoid intermediaite performance degradation on x86
2/ Change order
3/ Squash both patches together.

> 
> As a bonus the subject of the first patch makes actually sense
> then. Right now it does not because there is nothing to avoid :)
> 
> Also please use the same prefix for these two patches which touch the
> iter code.

Sure I'll do that.

> 
>> For the same reason as done in copy_from_user() by
>> commit 74e19ef0ff80 ("uaccess: Add speculation barrier to
>> copy_from_user()"), add a speculation barrier to copy_from_user_iter().
>>
>> See commit 74e19ef0ff80 ("uaccess: Add speculation barrier to
>> copy_from_user()") for more details.
> 
> No need to repeat that. Anyone with more than two braincells can look at
> that commit, which you mentioned already two lines above already.

Ok

Thanks
Christophe