[PATCH] x86: Allow user accesses to the base of the guard page

[PATCH] x86: Allow user accesses to the base of the guard page

[David Laight]

A user buffer can validly end with the last valid user address.
In that case access_ok(ptr, size) will check that 'ptr + size'
is a valid user address - and it needs to succeed.
access_ok() can't decrement the length because access_ok(ptr, 0)
also has to be valid.
Any actual access will fault.

Fixes: 86e6b1547b3d0 ("x86: fix user address masking non-canonical speculation issue")
Signed-off-by: David Laight <david.laight@aculab.com>
---
 arch/x86/kernel/cpu/common.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 06a516f6795b..ca327cfa42ae 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -2389,12 +2389,12 @@ void __init arch_cpu_finalize_init(void)
 	alternative_instructions();
 
 	if (IS_ENABLED(CONFIG_X86_64)) {
-		unsigned long USER_PTR_MAX = TASK_SIZE_MAX-1;
+		unsigned long USER_PTR_MAX = TASK_SIZE_MAX;
 
 		/*
 		 * Enable this when LAM is gated on LASS support
 		if (cpu_feature_enabled(X86_FEATURE_LAM))
-			USER_PTR_MAX = (1ul << 63) - PAGE_SIZE - 1;
+			USER_PTR_MAX = (1ul << 63) - PAGE_SIZE;
 		 */
 		runtime_const_init(ptr, USER_PTR_MAX);
 
-- 
2.17.1

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH] x86: Allow user accesses to the base of the guard page

[Linus Torvalds]

On Sat, 23 Nov 2024 at 10:48, David Laight <David.Laight@aculab.com> wrote:
>
> In that case access_ok(ptr, size) will check that 'ptr + size'
> is a valid user address -

The point of USER_PTR_MAX is that the size never matters and we never
check it. So the "-1" is basically just the minimal size.

And the code does actually depend on the fact that the access has to
start *before* the boundary to work.

Now, we do have that whole "at least PAGE_SIZE of guard page", and so
the 1-byte minimal size doesn't actually matter, but I don't see the
point of the change.

In particular, I don't see when it would matter to do access_ok(ptr,
0) in the first place. Who does that, and why would it make any sense?

               Linus

RE: [PATCH] x86: Allow user accesses to the base of the guard page

[David Laight]

From: Linus Torvalds
> Sent: 23 November 2024 19:03
> 
> On Sat, 23 Nov 2024 at 10:48, David Laight <David.Laight@aculab.com> wrote:
> >
> > In that case access_ok(ptr, size) will check that 'ptr + size'
> > is a valid user address -
> 
> The point of USER_PTR_MAX is that the size never matters and we never
> check it. So the "-1" is basically just the minimal size.
> 
> And the code does actually depend on the fact that the access has to
> start *before* the boundary to work.

That is the boundary at the end of the guard page.

> Now, we do have that whole "at least PAGE_SIZE of guard page", and so
> the 1-byte minimal size doesn't actually matter, but I don't see the
> point of the change.
> 
> In particular, I don't see when it would matter to do access_ok(ptr,
> 0) in the first place. Who does that, and why would it make any sense?

The problem is that it is valid to pass a buffer that ends right
at the end of valid user memory.
In that case the 'ptr + size' that access_ok() checks is equal to
'TASK_SIZE_MAX' - and currently fails.

There is also an access_ok() check in iovec_import (or is it
import_iovec) that does a check on every fragment.
It is definitely valid to pass a zero length buffer there.
(That check is probably redundant.)

So access_ok() can't check 'ptr + size - 1' without an extra check
for zero length.
And, in any case, you wouldn't want to subtract one in every access_ok()
call.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH] x86: Allow user accesses to the base of the guard page

[Linus Torvalds]

On Sat, 23 Nov 2024 at 14:36, David Laight <David.Laight@aculab.com> wrote:
>
> The problem is that it is valid to pass a buffer that ends right
> at the end of valid user memory.

There's a difference between "valid" and "we care".

This is way past that case. The only possible reason for that
zero-byte thing at the end of the address space is somebody actively
looking for some edge case, not a real use.

               Linus

Re: [PATCH] x86: Allow user accesses to the base of the guard page

[Mikel Rychliski]

On Saturday, November 23, 2024 6:44:34 P.M. EST you wrote:
> There's a difference between "valid" and "we care".
> 
> This is way past that case. The only possible reason for that
> zero-byte thing at the end of the address space is somebody actively
> looking for some edge case, not a real use.

access_ok() for x86_64 checks the validity of the byte one past the end of the 
requested buffer, even if that buffer is non-zero.

I ran into this in kernels that include 86e6b1547b3d0 with a BPF program that 
grabs the bottom of the user stack in PAGE_SIZE chunks. Reading the final page 
of user space returns -EFAULT now because the access_ok() check fails.

I've been working around with this:
https://lore.kernel.org/lkml/20241109210313.440495-1-mikel@mikelr.com/

Re: [PATCH] x86: Allow user accesses to the base of the guard page

[Linus Torvalds]

On Sat, 23 Nov 2024 at 16:24, Mikel Rychliski <mikel@mikelr.com> wrote:
>
> access_ok() for x86_64 checks the validity of the byte one past the end of the
> requested buffer, even if that buffer is non-zero.

Ahh, the non-zero case does indeed look like a valid reason for fixing
this. I was just reacting to the stated zero-sized case that really
doesn't matter.

So if David's patch fixes your use case, I'm happy to apply it with a
fixed commit message.

           Linus

Re: [PATCH] x86: Allow user accesses to the base of the guard page

[Mikel Rychliski]

On Saturday, November 23, 2024 7:30:06 P.M. EST Linus Torvalds wrote:
> So if David's patch fixes your use case, I'm happy to apply it with a
> fixed commit message.

Thanks, confirmed this case is fixed with David's patch:

Tested-by: Mikel Rychliski <mikel@mikelr.com>

RE: [PATCH] x86: Allow user accesses to the base of the guard page

[David Laight]

From: Linus Torvalds
> Sent: 23 November 2024 23:45
> 
> On Sat, 23 Nov 2024 at 14:36, David Laight <David.Laight@aculab.com> wrote:
> >
> > The problem is that it is valid to pass a buffer that ends right
> > at the end of valid user memory.
> 
> There's a difference between "valid" and "we care".
> 
> This is way past that case. The only possible reason for that
> zero-byte thing at the end of the address space is somebody actively
> looking for some edge case, not a real use.

Mikel gave the exact test that was failing.

I should have been more clear that the issue is fixing valid transfers
that end at the end of valid memory without breaking zero length
transfers anywhere else.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)