• Mark Rutland's avatar
    arm64: uaccess: correct thinko in __get_mem_asm() · f94511df
    Mark Rutland authored
    In the CONFIG_CC_HAS_ASM_GOTO_OUTPUT=y version of __get_mem_asm(), we
    incorrectly use _ASM_EXTABLE_##type##ACCESS_ERR() such that upon a fault
    the extable fixup handler writes -EFAULT into "%w0", which is the
    register containing 'x' (the result of the load).
    
    This was a thinko in commit:
    
      86a6a68f ("arm64: start using 'asm goto' for get_user() when available")
    
    Prior to that commit _ASM_EXTABLE_##type##ACCESS_ERR_ZERO() was used
    such that the extable fixup handler wrote -EFAULT into "%w0" (the
    register containing 'err'), and zero into "%w1" (the register containing
    'x'). When the 'err' variable was removed, the extable entry was updated
    incorrectly.
    
    Writing -EFAULT to the value register is unnecessary but benign:
    
    * We never want -EFAULT in the value register, and previously this would
      have been zeroed in the extable fixup handler.
    
    * In __get_user_error() the value is overwritten with zero explicitly in
      the error path.
    
    * The asm goto outputs cannot be used when the goto label is taken, as
      older compilers (e.g. clang < 16.0.0) do not guarantee that asm goto
      outputs are usable in this path and may use a stale value rather than
      the value in an output register. Consequently, zeroing in the extable
      fixup handler is insufficient to ensure callers see zero in the error
      path.
    
    * The expected usage of unsafe_get_user() and get_kernel_nofault()
      requires that the value is not consumed in the error path.
    
    Some versions of GCC would mis-compile asm goto with outputs, and
    erroneously omit subsequent assignments, breaking the error path
    handling in __get_user_error(). This was discussed at:
    
      https://lore.kernel.org/lkml/ZpfxLrJAOF2YNqCk@J2N7QTR9R3.cambridge.arm.com/
    
    ... and was fixed by removing support for asm goto with outputs on those
    broken compilers in commit:
    
      f2f6a8e8 ("init/Kconfig: remove CONFIG_GCC_ASM_GOTO_OUTPUT_WORKAROUND")
    
    With that out of the way, we can safely replace the usage of
    _ASM_EXTABLE_##type##ACCESS_ERR() with _ASM_EXTABLE_##type##ACCESS(),
    leaving the value register unchanged in the case a fault is taken, as
    was originally intended. This matches other architectures and matches
    our __put_mem_asm().
    Signed-off-by: default avatarMark Rutland <mark.rutland@arm.com>
    Cc: Will Deacon <will@kernel.org>
    Link: https://lore.kernel.org/r/20240807103731.2498893-1-mark.rutland@arm.comSigned-off-by: default avatarCatalin Marinas <catalin.marinas@arm.com>
    f94511df
uaccess.h 13.7 KB