[PATCH] arm64: sme: Use STR P to clear FFR context field in streaming SVE mode

[PATCH] arm64: sme: Use STR P to clear FFR context field in streaming SVE mode

[Will Deacon]

The FFR is a predicate register which can vary between 16 and 256 bits
in size depending upon the configured vector length. When saving the
SVE state in streaming SVE mode, the FFR register is inaccessible and
so commit 9f5848665788 ("arm64/sve: Make access to FFR optional") simply
clears the FFR field of the in-memory context structure. Unfortunately,
it achieves this using an unconditional 8-byte store and so if the SME
vector length is anything other than 64 bytes in size we will either
fail to clear the entire field or, worse, we will corrupt memory
immediately following the structure. This has led to intermittent kfence
splats in CI [1] and can trigger kmalloc Redzone corruption messages
when running the 'fp-stress' kselftest:

 | =============================================================================
 | BUG kmalloc-1k (Not tainted): kmalloc Redzone overwritten
 | -----------------------------------------------------------------------------
 |
 | 0xffff000809bf1e22-0xffff000809bf1e27 @offset=7714. First byte 0x0 instead of 0xcc
 | Allocated in do_sme_acc+0x9c/0x220 age=2613 cpu=1 pid=531
 |  __kmalloc+0x8c/0xcc
 |  do_sme_acc+0x9c/0x220
 |  ...

Replace the 8-byte store with a store of a predicate register which has
been zero-initialised with PFALSE, ensuring that the entire field is
cleared in memory.

[1] https://lore.kernel.org/r/CA+G9fYtU7HsV0R0dp4XEH5xXHSJFw8KyDf5VQrLLfMxWfxQkag@mail.gmail.com
Cc: Mark Brown <broonie@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Naresh Kamboju <naresh.kamboju@linaro.org>
Fixes: 9f5848665788 ("arm64/sve: Make access to FFR optional")
Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
Signed-off-by: Will Deacon <will@kernel.org>
---
 arch/arm64/include/asm/fpsimdmacros.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/fpsimdmacros.h b/arch/arm64/include/asm/fpsimdmacros.h
index cd03819a3b68..cdf6a35e3994 100644
--- a/arch/arm64/include/asm/fpsimdmacros.h
+++ b/arch/arm64/include/asm/fpsimdmacros.h
@@ -316,12 +316,12 @@
  _for n, 0, 15,	_sve_str_p	\n, \nxbase, \n - 16
 		cbz		\save_ffr, 921f
 		_sve_rdffr	0
-		_sve_str_p	0, \nxbase
-		_sve_ldr_p	0, \nxbase, -16
 		b		922f
 921:
-		str		xzr, [x\nxbase]		// Zero out FFR
+		_sve_pfalse	0			// Zero out FFR
 922:
+		_sve_str_p	0, \nxbase
+		_sve_ldr_p	0, \nxbase, -16
 		mrs		x\nxtmp, fpsr
 		str		w\nxtmp, [\xpfpsr]
 		mrs		x\nxtmp, fpcr
-- 
2.41.0.162.gfafddb0af9-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: sme: Use STR P to clear FFR context field in streaming SVE mode

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 871 bytes --]

On Wed, Jun 28, 2023 at 04:56:05PM +0100, Will Deacon wrote:
> The FFR is a predicate register which can vary between 16 and 256 bits
> in size depending upon the configured vector length. When saving the
> SVE state in streaming SVE mode, the FFR register is inaccessible and
> so commit 9f5848665788 ("arm64/sve: Make access to FFR optional") simply
> clears the FFR field of the in-memory context structure. Unfortunately,
> it achieves this using an unconditional 8-byte store and so if the SME
> vector length is anything other than 64 bytes in size we will either
> fail to clear the entire field or, worse, we will corrupt memory
> immediately following the structure. This has led to intermittent kfence
> splats in CI [1] and can trigger kmalloc Redzone corruption messages
> when running the 'fp-stress' kselftest:

Reviewed-by: Mark Brown <broonie@kernel.org>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: sme: Use STR P to clear FFR context field in streaming SVE mode

[Catalin Marinas]

On Wed, 28 Jun 2023 16:56:05 +0100, Will Deacon wrote:
> The FFR is a predicate register which can vary between 16 and 256 bits
> in size depending upon the configured vector length. When saving the
> SVE state in streaming SVE mode, the FFR register is inaccessible and
> so commit 9f5848665788 ("arm64/sve: Make access to FFR optional") simply
> clears the FFR field of the in-memory context structure. Unfortunately,
> it achieves this using an unconditional 8-byte store and so if the SME
> vector length is anything other than 64 bytes in size we will either
> fail to clear the entire field or, worse, we will corrupt memory
> immediately following the structure. This has led to intermittent kfence
> splats in CI [1] and can trigger kmalloc Redzone corruption messages
> when running the 'fp-stress' kselftest:
> 
> [...]

Applied to arm64 (for-next/core), thanks!

[1/1] arm64: sme: Use STR P to clear FFR context field in streaming SVE mode
      https://git.kernel.org/arm64/c/1c297ec19245

-- 
Catalin


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: sme: Use STR P to clear FFR context field in streaming SVE mode

[Anders Roxell]

On Wed, 28 Jun 2023 at 17:56, Will Deacon <will@kernel.org> wrote:
>
> The FFR is a predicate register which can vary between 16 and 256 bits
> in size depending upon the configured vector length. When saving the
> SVE state in streaming SVE mode, the FFR register is inaccessible and
> so commit 9f5848665788 ("arm64/sve: Make access to FFR optional") simply
> clears the FFR field of the in-memory context structure. Unfortunately,
> it achieves this using an unconditional 8-byte store and so if the SME
> vector length is anything other than 64 bytes in size we will either
> fail to clear the entire field or, worse, we will corrupt memory
> immediately following the structure. This has led to intermittent kfence
> splats in CI [1] and can trigger kmalloc Redzone corruption messages
> when running the 'fp-stress' kselftest:

Tested-by: Anders Roxell <anders.roxell@linaro.org>

I applied your patch on next-20230628 and ran the kselftest-arm64
tests over night...
I've not been able to reproduce "BUG: KFENCE: memory corruption in
fpsimd_release_task".

Cheers,
Anders

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel