All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Laight <David.Laight@ACULAB.COM>
To: 'Linus Torvalds' <torvalds@linux-foundation.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
	"bp@alien8.de" <bp@alien8.de>,
	Josh Poimboeuf <jpoimboe@kernel.org>,
	"x86@kernel.org" <x86@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Arnd Bergmann <arnd@kernel.org>,
	Mikel Rychliski <mikel@mikelr.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	"H. Peter Anvin" <hpa@zytor.com>
Subject: RE: [PATCH] x86: Allow user accesses to the base of the guard page
Date: Sat, 23 Nov 2024 22:36:34 +0000	[thread overview]
Message-ID: <e92823fee58d44b6a50a83fd27206857@AcuMS.aculab.com> (raw)
In-Reply-To: <CAHk-=wgT4E+6YO0-SvdD=Gh6wSuUB3Bq_qOiErfZkobQ9gn6=Q@mail.gmail.com>

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)

  reply	other threads:[~2024-11-23 22:36 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-23 18:48 [PATCH] x86: Allow user accesses to the base of the guard page David Laight
2024-11-23 19:02 ` Linus Torvalds
2024-11-23 22:36   ` David Laight [this message]
2024-11-23 23:44     ` Linus Torvalds
2024-11-24  0:24       ` Mikel Rychliski
2024-11-24  0:30         ` Linus Torvalds
2024-11-24  3:10           ` Mikel Rychliski
2024-11-24 10:28       ` David Laight

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e92823fee58d44b6a50a83fd27206857@AcuMS.aculab.com \
    --to=david.laight@aculab.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=arnd@kernel.org \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=jpoimboe@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mikel@mikelr.com \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.