Re: [PATCH 2/5] uaccess: Add speculation barrier to copy_from_user_iter()

[David Laight <david.laight.linux@gmail.com>]

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