Re: [PATCH] x86/uaccess: fix code generation in put_user()

From: hpa
Date: Fri Oct 23 2020 - 16:55:20 EST


On October 23, 2020 1:42:39 PM PDT, Andy Lutomirski <luto@xxxxxxxxxx> wrote:
>On Fri, Oct 23, 2020 at 1:32 PM Rasmus Villemoes
><linux@xxxxxxxxxxxxxxxxxx> wrote:
>>
>> Quoting
>> https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html:
>>
>> You can define a local register variable and associate it with a
>> specified register...
>>
>> The only supported use for this feature is to specify registers for
>> input and output operands when calling Extended asm (see Extended
>> Asm). This may be necessary if the constraints for a particular
>> machine don't provide sufficient control to select the desired
>> register.
>>
>> On 32-bit x86, this is used to ensure that gcc will put an 8-byte
>> value into the %edx:%eax pair, while all other cases will just use
>the
>> single register %eax (%rax on x86-64). While the _ASM_AX actually
>just
>> expands to "%eax", note this comment next to get_user() which does
>> something very similar:
>>
>> * The use of _ASM_DX as the register specifier is a bit of a
>> * simplification, as gcc only cares about it as the starting point
>> * and not size: for a 64-bit value it will use %ecx:%edx on 32 bits
>> * (%ecx being the next register in gcc's x86 register sequence), and
>> * %rdx on 64 bits.
>>
>> However, getting this to work requires that there is no code between
>> the assignment to the local register variable and its use as an input
>> to the asm() which can possibly clobber any of the registers involved
>> - including evaluation of the expressions making up other inputs.
>
>This looks like the patch is an improvement, but this is still IMO
>likely to be very fragile. Can we just do the size-dependent "a" vs
>"A" selection method instead? Sure, it's a little more code, but it
>will be Obviously Correct. As it stands, I can easily see our code
>failing on some gcc or clang version and the compiler authors telling
>us that we're relying on unsupportable behavior.

Yeah, the reason get_user hacks it is because there is no equivalent to "A" for other register pairs, but not using it for dx:ax is just silly.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.