| Message ID | 20210806135331.GA2951@willie-the-truck |
|---|---|
| State | New |
| Headers | show |
| Series | [GIT,PULL] arm64 fixes for -rc5 | expand |
On Fri, Aug 6, 2021 at 6:53 AM Will Deacon <will@kernel.org> wrote: > > Please pull these arm64 fixes for -rc5. It's all pretty minor but the > main fix is sorting out how we deal with return values from 32-bit system > calls as audit expects error codes to be sign-extended to 64 bits I've pulled this, but that change looks _really_ odd. First you seem to intentionally *zero-extend* the error value when you actually set it in pt_regs, and then you sign-extend them when reading them. So the rules seem entirely arbitrary: oen place says "upper 32 bits need to be clear" and another place says "upper 32 bits need to be sign-extended". Why this insanity? Why not make the rule be that the upper 32 bits are always just sign-extended? Linus
The pull request you sent on Fri, 6 Aug 2021 14:53:32 +0100:
> git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git tags/arm64-fixes
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/73f25536f27182ae3dcf4c0b91b1280cbbac7be3
Thank you!
Hi Linus, [+Mark] On Fri, Aug 06, 2021 at 11:40:33AM -0700, Linus Torvalds wrote: > On Fri, Aug 6, 2021 at 6:53 AM Will Deacon <will@kernel.org> wrote: > > > > Please pull these arm64 fixes for -rc5. It's all pretty minor but the > > main fix is sorting out how we deal with return values from 32-bit system > > calls as audit expects error codes to be sign-extended to 64 bits > > I've pulled this, but that change looks _really_ odd. Cheers, and yes it does. We're stuck in the middle of the architecture, the compat ABI and internal kernel expectations. More below. > First you seem to intentionally *zero-extend* the error value when you > actually set it in pt_regs, and then you sign-extend them when reading > them. > > So the rules seem entirely arbitrary: oen place says "upper 32 bits > need to be clear" and another place says "upper 32 bits need to be > sign-extended". > > Why this insanity? Why not make the rule be that the upper 32 bits are > always just sign-extended? There are a few things which collide here: The architecture doesn't guarantee that the upper 32-bits of a 64-bit general purpose register are preserved across an exception return to a 32-bit task. They _might_ be left intact, but it's up to the CPU whether you get the value you wrote or all zeroes if you read those bits after taking an exception back to 64-bit state. Consequently, we can't expose 64-bit registers for 32-bit tasks via ptrace() as the resulting behaviour is going to vary based on how the hardware feels. Maybe we could sign-extend everything on exception entry, but that would necessitate many more syscall wrappers for compat tasks than we currently have so we could truncate pointer arguments back down to 32 bits. Instead, we currently handle this by (a) treating the registers of a 32-bit task as 32 bits (hence the zero extension when writing the value in syscall_set_return_value()) and (b) explicitly clearing the upper bits of x0 on exception entry from a 32-bit task in case we previously leaked a negative syscall return value in there. The problem then is that some in-kernel users (e.g. audit and some parts of ptrace which abstract the syscall return value away from the register state) _do_ want to see sign-extended syscall return arguments in order to match against error codes (see is_syscall_success()). So we end up in a situation where we need to sign-extend the return value for those, whilst leaving the zero-extended version in the actual pt_regs structure. It's ugly and subtle because the sky doesn't tend to fall in if you get it wrong. As you can see, we're still fixing it. Will
Hi Linus, Please pull these arm64 fixes for -rc5. It's all pretty minor but the main fix is sorting out how we deal with return values from 32-bit system calls as audit expects error codes to be sign-extended to 64 bits Brief summary in the tag. Cheers, Will --->8 The following changes since commit d8a719059b9dc963aa190598778ac804ff3e6a87: Revert "mm/pgtable: add stubs for {pmd/pub}_{set/clear}_huge" (2021-07-21 11:28:09 +0100) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git tags/arm64-fixes for you to fetch changes up to 0c32706dac1b0a72713184246952ab0f54327c21: arm64: stacktrace: avoid tracing arch_stack_walk() (2021-08-03 10:39:35 +0100) ---------------------------------------------------------------- arm64 fixes for -rc5 - Fix extension/truncation of return values from 32-bit system calls - Fix interaction between unwinding and tracing - Fix spurious toolchain warning emitted during make - Fix Kconfig help text for RANDOMIZE_MODULE_REGION_FULL ---------------------------------------------------------------- Barry Song (1): arm64: fix the doc of RANDOMIZE_MODULE_REGION_FULL Mark Rutland (3): arm64: fix compat syscall return truncation arm64: stacktrace: fix comment arm64: stacktrace: avoid tracing arch_stack_walk() Masahiro Yamada (1): arm64: move warning about toolchains to archprepare arch/arm64/Kconfig | 9 ++++++--- arch/arm64/Makefile | 21 ++++++++++++--------- arch/arm64/include/asm/ptrace.h | 12 +++++++++++- arch/arm64/include/asm/stacktrace.h | 2 +- arch/arm64/include/asm/syscall.h | 19 ++++++++++--------- arch/arm64/kernel/kaslr.c | 4 +++- arch/arm64/kernel/ptrace.c | 2 +- arch/arm64/kernel/signal.c | 3 ++- arch/arm64/kernel/stacktrace.c | 2 +- arch/arm64/kernel/syscall.c | 9 +++------ 10 files changed, 50 insertions(+), 33 deletions(-)