linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Laight <David.Laight@ACULAB.COM>
To: 'Andy Lutomirski' <luto@kernel.org>,
	Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>, X86 ML <x86@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Will Deacon <will@kernel.org>,
	Dan Williams <dan.j.williams@intel.com>,
	Andrea Arcangeli <aarcange@redhat.com>,
	"Waiman Long" <longman@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Christoph Hellwig <hch@lst.de>,
	Mark Rutland <mark.rutland@arm.com>,
	"Borislav Petkov" <bp@alien8.de>
Subject: RE: [PATCH v4 3/4] x86/uaccess: Use pointer masking to limit uaccess speculation
Date: Thu, 6 May 2021 08:36:08 +0000	[thread overview]
Message-ID: <986301bcdc7c488d86dd5f11c988bf87@AcuMS.aculab.com> (raw)
In-Reply-To: <CALCETrWQrMkeP+=pkNVNvSs9q3ZhhLq_ceHJ-N72Urp2KBrUfQ@mail.gmail.com>

From: Andy Lutomirski
> Sent: 05 May 2021 17:55
...
> Is there an equally efficient sequence that squishes the pointer value
> to something noncanonical or something like -1 instead of 0?  I'm not
> sure this matters, but it opens up the possibility of combining the
> access_ok check with the masking without any branches at all.

Are you thinking of using:
	uaddr = access_ok(uaddr, size)
and having the output value being one that is guaranteed
to fault when (a little later on) used to access user memory?

As well as the problem of finding a suitable invalid address
in 32bit architectures there can be issues if the code accesses
(uaddr + big_offset) since that could be outside the invalid
address window.

We are back to the fact that if we know the accesses are
sequential (or a single access) then it can usually be
arranged for them to fault without an explicit size check.

This could mean you have:
	if (access_ok_mask(&uaddr, size))
		return -EFAULT;
that never actually returns EFAULT on some architectures
when size is a small compile-time constant.

If you don't need to check the size then you'd need
something like:
	mov uaddr, reg
	add #-TASK_SIZE_MAX, reg	// sets carry for bad addresses
	sbb reg, reg			// -1 for bad addresses
	or  reg, uaddr
That converts addresses above TASK_SIZE_MASK to -1.
Non-byte accesses will fault on all x86 cpu.
For x64 (and some other 64bit) you can clear the top few
bits to get an invalid address.

So probably ok for get_user() and copy_from_user() (etc)
but not as a more general check.

	David.

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

  reply	other threads:[~2021-05-06  8:36 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-05  3:54 [PATCH v4 0/4] x86/uaccess: Use pointer masking to limit uaccess speculation Josh Poimboeuf
2021-05-05  3:54 ` [PATCH v4 1/4] uaccess: Always inline strn*_user() helper functions Josh Poimboeuf
2021-05-05  3:54 ` [PATCH v4 2/4] uaccess: Fix __user annotations for copy_mc_to_user() Josh Poimboeuf
2021-05-05  3:54 ` [PATCH v4 3/4] x86/uaccess: Use pointer masking to limit uaccess speculation Josh Poimboeuf
2021-05-05  8:48   ` David Laight
2021-05-05 13:19     ` Josh Poimboeuf
2021-05-05 13:51       ` David Laight
2021-05-05 18:32     ` Linus Torvalds
2021-05-06  7:57       ` David Laight
2021-05-05 14:25   ` Mark Rutland
2021-05-05 14:48     ` Josh Poimboeuf
2021-05-05 14:49     ` David Laight
2021-05-05 15:45       ` Mark Rutland
2021-05-05 16:55   ` Andy Lutomirski
2021-05-06  8:36     ` David Laight [this message]
2021-05-06 12:05       ` Christoph Hellwig
2021-06-02 17:11   ` Sean Christopherson
2021-06-02 20:11     ` Josh Poimboeuf
2021-05-05  3:54 ` [PATCH v4 4/4] x86/nospec: Remove barrier_nospec() Josh Poimboeuf

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=986301bcdc7c488d86dd5f11c988bf87@AcuMS.aculab.com \
    --to=david.laight@aculab.com \
    --cc=aarcange@redhat.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=bp@alien8.de \
    --cc=dan.j.williams@intel.com \
    --cc=hch@lst.de \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=luto@kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=will@kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).