Message-ID: <b853710c4cb94ec0bf869edb41a685b6@AcuMS.aculab.com>
Date: Sun, 1 Dec 2024 22:24:12 +0000
From: David Laight <David.Laight@...LAB.COM>
To: 'Linus Torvalds' <torvalds@...uxfoundation.org>
CC: "x86@...nel.org" <x86@...nel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, Thomas Gleixner <tglx@...utronix.de>, "Ingo
 Molnar" <mingo@...hat.com>, Dave Hansen <dave.hansen@...ux.intel.com>,
	"Andrew Cooper" <andrew.cooper3@...rix.com>, Josh Poimboeuf
	<jpoimboe@...nel.org>, "bp@...en8.de" <bp@...en8.de>
Subject: RE: [PATCH next] x86: mask_user_address() return base of guard page
 for kernel addresses

From: Linus Torvalds
> Sent: 01 December 2024 20:03
> 
> On Sun, 1 Dec 2024 at 10:12, David Laight <David.Laight@...lab.com> wrote:
> >
> > I've built and run a kernel with it - so not broken!
> 
> I worry that 'cmov' could be predicted - making the whole sequence
> pointless. It would be a stupid thing for a CPU core to do, but it
> would be simple.

Not really, 'prediction' is all about the where the 'front end' reads
instructions from - and trying to avoid it depending on other cpu state.
Which is why some cpu don't even check the instruction is even a branch.

Whereas 'cmov' is pretty much an ALU instruction with the output
depending on the state of registers.

Agner's tables pretty much show that Intel implemented as
	x = cond ? y : x
so it suffers from being a 2 u-op instruction (the same as sbb)
on older core-2 cpu.
OTOH AMD have is as '4 per clock' (the same as mov) so could be
a 'mov' with the write disabled' (but I'm not sure how that
would work if 'mov' is a register rename).

> Of course, 'sbb' could be done using predicting the carry flag too.
> There's a lot of ways to screw this up.
> 
> Intel at some point explicitly said
> 
>  "Other instructions such as CMOVcc, AND, ADC, SBB and SETcc can also

Presumably that is 'as well an lfence' ?
Not sure how much use setcc could be - it only changes %al.

> be used to prevent bounds
>   check bypass by constraining speculative execution on current family
> 6 processors (Intel® Core™,
>   Intel® Atom™, Intel® Xeon® and Intel® Xeon Phi™ processors).
> However, these instructions may not
>   be guaranteed to do so on future Intel processors"
> 
> so none of these are safe according to that.

Well, they are all currently safe, but the future is undecided.
Sounds like just be non-committal...

> Maybe there were newer updates on this, but in the meantime I'd rather
> have just *one* pattern, not switch between multiple possibly
> problematic ones. And sbb has been that traditional one.

Well, array_index_nospec() could just be a cmp + cmov.
If you pass 'size - 1' that can be used for the 'out of range' value.
Otherwise you'd need to pass in a zero (that the compiler can generate earlier).
That would save an instruction (and register dependency) in the 'index in range' path.

The only other use I spotted is in files_lookup_fd_raw() which also uses the
0/~1 from array_index_mask_nospec() to also mask the value read from xxx[0].

> Also, if sbb is ever made speculative, I think it's time to just jump ship.

Well AND and (presumably) OR are also in the list.
You are relying on those as well as the sbb.

If you get issues in a mips-like cpu it all gets harder.
Without a 'carry flag' you can user 'sbb'.
It also makes any kind of cmov hard to implement.
I guess cmov-odd and cmov-even could be implemented by disabling
the register-file write (and used after a cmp - that sets 0/1).
Whether the architecture defines/implements them is another matter.
They aren't in the one I implemented earlier in the year.

	David

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

Powered by blists - more mailing lists