Messages in this thread | | | From | Linus Torvalds <> | Date | Mon, 12 Oct 2020 11:11:35 -0700 | Subject | Re: [GIT PULL] x86/asm updates for v5.10 |
| |
On Mon, Oct 12, 2020 at 4:06 AM Borislav Petkov <bp@suse.de> wrote: > > * Use XORL instead of XORQ to avoid a REX prefix and save some bytes in > the .fixup section, by Uros Bizjak.
I think this one is actually buggy.
For the 1-byte case, it does this:
__get_user_asm(x_u8__, ptr, retval, "b", "=q");
and ends up doing "xorl" on a register that we told the compiler is a byte register (with that "=q")
Yes, it uses "%k[output]" to turn that byte register into the word version of the register, but there's no fundamental reason why the register might not be something like "%ah".
Does the "xorl" work? Does it build? Yes, and yes.
But maybe %al contains SOMETHING ELSE, and it now clears that too, because the asm is basically doing something completely different than what we told the compiler it would do.
Now, afaik, gcc (and presumably clang) basically almost never use the high byte registers. But I still think this patch is fundamentally wrong and conceptually completely buggy, even if it might work in practice.
Also, I'm going to uninline this nasty __get_user() function anyway for 5.10, so the patch ends up being not just wrong, but pointless. This is not some kind of hot code that should be optimized, and the extra byte is not a lot to worry about.
Annoying. Because the other patch in this pull request is fine, and people want it.
But I'm going to skip this pull request, because I really think it's dangerously and subtly buggy even if there might not be any case that matters in reality.
Linus
| |