mbox series

[GIT,PULL] arm64 fixes for -rc5

Message ID 20210806135331.GA2951@willie-the-truck (mailing list archive)
State New, archived
Headers show
Series [GIT,PULL] arm64 fixes for -rc5 | expand

Pull-request

git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git tags/arm64-fixes

Message

Will Deacon Aug. 6, 2021, 1:53 p.m. UTC
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(-)

Comments

Linus Torvalds Aug. 6, 2021, 6:40 p.m. UTC | #1
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
pr-tracker-bot@kernel.org Aug. 6, 2021, 6:48 p.m. UTC | #2
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!
Will Deacon Aug. 9, 2021, 10:52 a.m. UTC | #3
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