Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

x86/uaccess: fix code generation in put_user()

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.

In the current code, the ptr expression used directly as an input may
cause such code to be emitted. For example, Sean Christopherson
observed that with KASAN enabled and ptr being current->set_child_tid
(from chedule_tail()), the load of current->set_child_tid causes a call
to __asan_load8() to be emitted immediately prior to the __put_user_4
call, and Naresh Kamboju reports that various mmstress tests fail on
KASAN-enabled builds.

It's also possible to synthesize a broken case without KASAN if one uses
"foo()" as the ptr argument, with foo being some "extern u64 __user
*foo(void);" (though I don't know if that appears in real code).

Fix it by making sure ptr gets evaluated before the assignment to
__val_pu, and add a comment that __val_pu must be the last thing
computed before the asm() is entered.

Cc: Sean Christopherson <sean.j.christopherson@intel.com>
Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Tested-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Fixes: d55564cfc222 ("x86: Make __put_user() generate an out-of-line call")
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

authored by

Rasmus Villemoes and committed by
Linus Torvalds
9c5743df 3cb12d27

+9 -1
+9 -1
arch/x86/include/asm/uaccess.h
··· 208 208 extern void __put_user_nocheck_4(void); 209 209 extern void __put_user_nocheck_8(void); 210 210 211 + /* 212 + * ptr must be evaluated and assigned to the temporary __ptr_pu before 213 + * the assignment of x to __val_pu, to avoid any function calls 214 + * involved in the ptr expression (possibly implicitly generated due 215 + * to KASAN) from clobbering %ax. 216 + */ 211 217 #define do_put_user_call(fn,x,ptr) \ 212 218 ({ \ 213 219 int __ret_pu; \ 220 + void __user *__ptr_pu; \ 214 221 register __typeof__(*(ptr)) __val_pu asm("%"_ASM_AX); \ 215 222 __chk_user_ptr(ptr); \ 223 + __ptr_pu = (ptr); \ 216 224 __val_pu = (x); \ 217 225 asm volatile("call __" #fn "_%P[size]" \ 218 226 : "=c" (__ret_pu), \ 219 227 ASM_CALL_CONSTRAINT \ 220 - : "0" (ptr), \ 228 + : "0" (__ptr_pu), \ 221 229 "r" (__val_pu), \ 222 230 [size] "i" (sizeof(*(ptr))) \ 223 231 :"ebx"); \