Re: [PATCH] x86_64, asm: Work around AMD SYSRET SS descriptor attribute issue

Borislav Petkov
2015-04-27T15:46:51Z

On Mon, Apr 27, 2015 at 07:57:36AM -0700, Linus Torvalds wrote:
> On Mon, Apr 27, 2015 at 4:35 AM, Borislav Petkov <bp@alien8.de> wrote:
> >
> > /*
> > * Change top 16 bits to be the sign-extension of 47th bit, if this
> > * changed %rcx, it was not canonical.
> > */
> > ALTERNATIVE "", \
> > "shl $(64 - (47+1)), %rcx; \
> > sar $(64 - (47+1)), %rcx; \
> > cmpq %rcx, %r11; \
> > jne opportunistic_sysret_failed", X86_BUG_SYSRET_CANON_RCX
>
> Guys, if we're looking at cycles for this, then don't do the "exact
> canonical test". and go back to just doing
>
> shr $__VIRTUAL_MASK_SHIFT, %rcx
> jnz opportunistic_sysret_failed
>
> which is much smaller.

Right, what about the false positives:

17be0aec74fb ("x86/asm/entry/64: Implement better check for canonical addresses")

? We don't care?

> In fact, aim to make the conditional jump be a
> two-byte one (jump forward to another jump if required - it's a
> slow-path that doesn't matter at *all* for the taken case), and the
> end result is just six bytes. That way you can use alternative to
> replace it with one single noop on AMD.

Well, even with the non-optimal NOPs (we end up with 4 3-byte NOPs and
one single-byte), we're still better than the unconditional JMP I had
there before:

https://lkml.kernel.org/r/20150427143905.GK6774@pd.tnic

(you might want to look at the raw email - marc.info breaks lines)

I'll retest with the F16h NOPs to see whether there's any difference.

Thanks.

--
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
--