On Thu, Nov 21, 2024 at 02:16:12PM -0800, Linus Torvalds wrote:
> mov %gs:0x0,%rax # current
> incl 0x1a9c(%rax) # current->pagefault_disable++
> movabs $0x123456789abcdef,%rcx # magic virtual address size
> cmp %rsi,%rcx # address masking
> sbb %rcx,%rcx
> or %rsi,%rcx
> stac # enable user space acccess
> mov (%rcx),%ecx # get the value
> clac # disable user space access
> decl 0x1a9c(%rax) # current->pagefault_disable--
> mov %ecx,(%rdi) # save the value
> xor %eax,%eax # return 0
> ret
The asm looks good, but the C exploded a bit... why not just have an
inline get_user()?
> If you can test this and verify that it actually help, I'll take it as
> a patch. Consider it signed-off after testing.
Let me see if I can recreate the original report (or get the automatic
testing to see the commit).
> > Also, is there any harm in speeding up __get_user()? It still has ~80
> > callers and it's likely to be slowing down things we don't know about.
>
> How would you speed it up? We definitely can't replace the fence with
> addressing tricks. So we can't just replace it with "get_user()",
> because of those horrid architecture-specific kernel uses.
I'm not sure if you saw the example code snippet I posted up-thread,
here it is below.
It adds a conditional branch to test if it's a user address. If so, it
does pointer masking. If not, it does LFENCE. So "bad" users get the
slow path, and we could even add a WARN_ON().
Yes, another conditional branch isn't ideal, but it's still much faster
than the current code, and it would root out any bad users with a
WARN_ON() so that eventually it can just become a get_user() alias.
.macro __get_user_nocheck_nospec
#ifdef CONFIG_X86_64
movq $0x0123456789abcdef, %rdx
1:
.pushsection runtime_ptr_USER_PTR_MAX, "a"
.long 1b - 8 - .
.popsection
cmp %rax, %rdx
jb 10f
sbb %rdx, %rdx
or %rdx, %rax
jmp 11f
10: /*
* Stop access_ok() branch misprediction -- both of them ;-)
*
* As a benefit this also punishes callers who intentionally call this
* with a kernel address. Once they're rooted out, __get_user() can
* just become an alias of get_user().
*
* TODO: Add WARN_ON()
*/
#endif
ASM_BARRIER_NOSPEC
11:
.endm
/* .. and the same for __get_user, just without the range checks */
SYM_FUNC_START(__get_user_nocheck_1)
__get_user_nocheck_nospec
ASM_STAC
UACCESS movzbl (%_ASM_AX),%edx
xor %eax,%eax
ASM_CLAC
RET
SYM_FUNC_END(__get_user_nocheck_1)
EXPORT_SYMBOL(__get_user_nocheck_1)
--
Josh
