mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-04-05 11:55:31 -04:00
3eb6660f26
ASM GOTO is miscompiled by GCC when it is used inside a auto cleanup scope:
bool foo(u32 __user *p, u32 val)
{
scoped_guard(pagefault)
unsafe_put_user(val, p, efault);
return true;
efault:
return false;
}
e80: e8 00 00 00 00 call e85 <foo+0x5>
e85: 65 48 8b 05 00 00 00 00 mov %gs:0x0(%rip),%rax
e8d: 83 80 04 14 00 00 01 addl $0x1,0x1404(%rax) // pf_disable++
e94: 89 37 mov %esi,(%rdi)
e96: 83 a8 04 14 00 00 01 subl $0x1,0x1404(%rax) // pf_disable--
e9d: b8 01 00 00 00 mov $0x1,%eax // success
ea2: e9 00 00 00 00 jmp ea7 <foo+0x27> // ret
ea7: 31 c0 xor %eax,%eax // fail
ea9: e9 00 00 00 00 jmp eae <foo+0x2e> // ret
which is broken as it leaks the pagefault disable counter on failure.
Clang at least fails the build.
Linus suggested to add a local label into the macro scope and let that
jump to the actual caller supplied error label.
__label__ local_label; \
arch_unsafe_get_user(x, ptr, local_label); \
if (0) { \
local_label: \
goto label; \
That works for both GCC and clang.
clang:
c80: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1)
c85: 65 48 8b 0c 25 00 00 00 00 mov %gs:0x0,%rcx
c8e: ff 81 04 14 00 00 incl 0x1404(%rcx) // pf_disable++
c94: 31 c0 xor %eax,%eax // set retval to false
c96: 89 37 mov %esi,(%rdi) // write
c98: b0 01 mov $0x1,%al // set retval to true
c9a: ff 89 04 14 00 00 decl 0x1404(%rcx) // pf_disable--
ca0: 2e e9 00 00 00 00 cs jmp ca6 <foo+0x26> // ret
The exception table entry points correctly to c9a
GCC:
f70: e8 00 00 00 00 call f75 <baz+0x5>
f75: 65 48 8b 05 00 00 00 00 mov %gs:0x0(%rip),%rax
f7d: 83 80 04 14 00 00 01 addl $0x1,0x1404(%rax) // pf_disable++
f84: 8b 17 mov (%rdi),%edx
f86: 89 16 mov %edx,(%rsi)
f88: 83 a8 04 14 00 00 01 subl $0x1,0x1404(%rax) // pf_disable--
f8f: b8 01 00 00 00 mov $0x1,%eax // success
f94: e9 00 00 00 00 jmp f99 <baz+0x29> // ret
f99: 83 a8 04 14 00 00 01 subl $0x1,0x1404(%rax) // pf_disable--
fa0: 31 c0 xor %eax,%eax // fail
fa2: e9 00 00 00 00 jmp fa7 <baz+0x37> // ret
The exception table entry points correctly to f99
So both compilers optimize out the extra goto and emit correct and
efficient code.
Provide a generic wrapper to do that to avoid modifying all the affected
architecture specific implementation with that workaround.
The only change required for architectures is to rename unsafe_*_user() to
arch_unsafe_*_user(). That's done in subsequent changes.
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Link: https://patch.msgid.link/877bweujtn.ffs@tglx