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().

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 | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index a589935bf3025..896760bad455f 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -49,11 +49,19 @@ size_t copy_from_user_iter(void __user *iter_from, size_t progress,
 
 	if (should_fail_usercopy())
 		return len;
-	if (can_do_masked_user_access())
+	if (can_do_masked_user_access()) {
 		iter_from = mask_user_address(iter_from);
-	else if (!access_ok(iter_from, len))
-		return res;
+	} else {
+		if (!access_ok(iter_from, len))
+			return res;
 
+		/*
+		 * 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);
-- 
2.49.0

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.

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.

> 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.

Thanks,

        tglx

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