linux-kernel - Re: [PATCH 2/5] 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: <20250622175238.642d02bf@pumpkin>
Date: Sun, 22 Jun 2025 17:52:38 +0100
From: David Laight <david.laight.linux@...il.com>
To: Christophe Leroy <christophe.leroy@...roup.eu>
Cc: Michael Ellerman <mpe@...erman.id.au>, Nicholas Piggin
 <npiggin@...il.com>, Naveen N Rao <naveen@...nel.org>, Madhavan Srinivasan
 <maddy@...ux.ibm.com>, Alexander Viro <viro@...iv.linux.org.uk>, Christian
 Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>, Thomas Gleixner
 <tglx@...utronix.de>, 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>, Dave Hansen
 <dave.hansen@...ux.intel.com>, Linus Torvalds
 <torvalds@...ux-foundation.org>, linux-kernel@...r.kernel.org,
 linuxppc-dev@...ts.ozlabs.org, linux-fsdevel@...r.kernel.org,
 linux-mm@...ck.org
Subject: Re: [PATCH 2/5] uaccess: Add speculation barrier to
 copy_from_user_iter()

On Sun, 22 Jun 2025 11:52:40 +0200
Christophe Leroy <christophe.leroy@...roup.eu> 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
> 
> 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().

I'm sure I sent a patch to change this code to used the 'masked' functions.
Probably ought to be done at the same time.
Would have been early feb, about the time I suggested:

+#ifdef masked_user_access_begin
+#define masked_user_read_access_begin(from, size) \
+	((*(from) = masked_user_access_begin(*(from))), 1)
+#define masked_user_write_access_begin(from, size) \
+	((*(from) = masked_user_access_begin(*(from))), 1)
+#else
+#define masked_user_read_access_begin(from, size) \
+	user_read_access_begin(*(from), size)
+#define masked_user_write_access_begin(from, size) \
+	user_write_access_begin(*(from), size)
+#endif

allowing:
-		if (!user_read_access_begin(from, sizeof(*from)))
+		if (!masked_user_read_access_begin(&from, sizeof(*from)))

	David


> 
> See commit 74e19ef0ff80 ("uaccess: Add speculation barrier to
> copy_from_user()") for more details.
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@...roup.eu>
> ---
>  lib/iov_iter.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/lib/iov_iter.c b/lib/iov_iter.c
> index f9193f952f49..ebf524a37907 100644
> --- a/lib/iov_iter.c
> +++ b/lib/iov_iter.c
> @@ -50,6 +50,13 @@ size_t copy_from_user_iter(void __user *iter_from, size_t progress,
>  	if (should_fail_usercopy())
>  		return len;
>  	if (access_ok(iter_from, len)) {
> +		/*
> +		 * Ensure that bad access_ok() speculation will not
> +		 * lead to nasty side effects *after* the copy is
> +		 * finished:
> +		 */
> +		barrier_nospec();
> +
>  		to += progress;
>  		instrument_copy_from_user_before(to, iter_from, len);
>  		res = raw_copy_from_user(to, iter_from, len);