| Message ID | 20241205121655.1824269-1-mark.rutland@arm.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | arm64: ptrace: fix handling of partial SETREGSET calls | expand |
On Thu, Dec 05, 2024 at 12:16:51PM +0000, Mark Rutland wrote: > A few of arm64's regsets forget to handle partial-length SETREGSET > calls, and copy a small portion of uninitialized kernel stack memory to > the backing storage of the relevant registers. In all cases the read is > limited to a specific slot on the stack, and the issue does not provide > a write mechanism. It feels like we might want a helpers specifically for the case where we're mapping a value directly into a data structure, it all feels a bit open coding. The shape is a bit awkward though since things aren't usually stored in the same layout as we expose via ptrace when there's more than one value. Reviewed-by: Mark Brown <broonie@kernel.org>
On Thu, 05 Dec 2024 12:16:51 +0000, Mark Rutland wrote: > A few of arm64's regsets forget to handle partial-length SETREGSET > calls, and copy a small portion of uninitialized kernel stack memory to > the backing storage of the relevant registers. In all cases the read is > limited to a specific slot on the stack, and the issue does not provide > a write mechanism. > > For example, a zero-length SETREGSET to NT_ARM_FPMR will reset FPMR to > an arbitrary uninitialized value from the kernel stack: > > [...] Applied to arm64 (for-next/fixes), thanks! [1/4] arm64: ptrace: fix partial SETREGSET for NT_ARM_TAGGED_ADDR_CTRL https://git.kernel.org/arm64/c/ca62d90085f4 [2/4] arm64: ptrace: fix partial SETREGSET for NT_ARM_FPMR https://git.kernel.org/arm64/c/f5d71291841a [3/4] arm64: ptrace: fix partial SETREGSET for NT_ARM_POE https://git.kernel.org/arm64/c/594bfc4947c4 [4/4] arm64: ptrace: fix partial SETREGSET for NT_ARM_GCS https://git.kernel.org/arm64/c/d60624f72d15
A few of arm64's regsets forget to handle partial-length SETREGSET calls, and copy a small portion of uninitialized kernel stack memory to the backing storage of the relevant registers. In all cases the read is limited to a specific slot on the stack, and the issue does not provide a write mechanism. For example, a zero-length SETREGSET to NT_ARM_FPMR will reset FPMR to an arbitrary uninitialized value from the kernel stack: | # ./fpmr-test | Attempting to write NT_ARM_FPMR::fpmr = 0x900d900d900d900d | SETREGSET(nt=0x40e, len=8) wrote 8 bytes | | Attempting to read NT_ARM_FPMR::fpmr | GETREGSET(nt=0x40e, len=8) read 8 bytes | Read NT_ARM_FPMR::fpmr = 0x900d900d900d900d | | Attempting to write NT_ARM_FPMR (zero length) | SETREGSET(nt=0x40e, len=0) wrote 0 bytes | | Attempting to read NT_ARM_FPMR::fpmr | GETREGSET(nt=0x40e, len=8) read 8 bytes | Read NT_ARM_FPMR::fpmr = 0xffff800083963d50 This series (based on v6.13-rc1) fixes affected regsets. To fix each case there are three approaches we could follow: (a) Preserve the registers which are not explicitly written to. Most other regsets (e.g. NT_PRSTATUS, NT_PRFPREG, NT_ARM_SYSTEM_CALL) follow this approach. (b) Reset registers to a fixed value (e.g. zero) if not explicitly written to. (c) Forbid writes that are smaller than the entire regset. For example, x86 does this in xfpregs_set(). For most regsets, partial-length SETREGSET calls follow option (a) today, so I've taken that approach for all the affected arm64 regsets. I've checked all of arm64's regsets (native, native view of compat, and compat view of compat), and as far as I can tell, the only affected regsets are NT_ARM_TAGGED_ADDR_CTRL, NT_ARM_FPMR, NT_ARM_POE, and NT_ARM_GCS. All other regsets either either initialize a temporary buffer prior to copying in, or copy directly to the backing storage, and so don't copy uninitialized data. There are some other cleanups which could be made here in future; ideally we'd have a compiler warning for these cases, or a mechanism that avoids the problem entirely. Mark. Mark Rutland (4): arm64: ptrace: fix partial SETREGSET for NT_ARM_TAGGED_ADDR_CTRL arm64: ptrace: fix partial SETREGSET for NT_ARM_FPMR arm64: ptrace: fix partial SETREGSET for NT_ARM_POE arm64: ptrace: fix partial SETREGSET for NT_ARM_GCS arch/arm64/kernel/ptrace.c | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-)