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

David Laight
2025-06-22T16:52:42Z

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);