[PATCH 03/15] crypto: arm64/aes - Fix 32-bit aes_mac_update() arg treated as 64-bit

[PATCH 03/15] crypto: arm64/aes - Fix 32-bit aes_mac_update() arg treated as 64-bit

[Eric Biggers]

Since the 'enc_after' argument to neon_aes_mac_update() and
ce_aes_mac_update() has type 'int', it needs to be accessed using the
corresponding 32-bit register, not the 64-bit register.  The upper half
of the corresponding 64-bit register may contain garbage.

Fixes: 4860620da7e5 ("crypto: arm64/aes - add NEON/Crypto Extensions CBCMAC/CMAC/XCBC driver")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 arch/arm64/crypto/aes-modes.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/crypto/aes-modes.S b/arch/arm64/crypto/aes-modes.S
index 0e834a2c062c..e793478f37c1 100644
--- a/arch/arm64/crypto/aes-modes.S
+++ b/arch/arm64/crypto/aes-modes.S
@@ -836,11 +836,11 @@ AES_FUNC_START(aes_mac_update)
 	encrypt_block	v0, w2, x1, x7, w8
 	eor		v0.16b, v0.16b, v3.16b
 	encrypt_block	v0, w2, x1, x7, w8
 	eor		v0.16b, v0.16b, v4.16b
 	cmp		w3, wzr
-	csinv		x5, x6, xzr, eq
+	csinv		w5, w6, wzr, eq
 	cbz		w5, .Lmacout
 	encrypt_block	v0, w2, x1, x7, w8
 	st1		{v0.16b}, [x4]			/* return dg */
 	cond_yield	.Lmacout, x7, x8
 	b		.Lmacloop4x
@@ -850,11 +850,11 @@ AES_FUNC_START(aes_mac_update)
 	cbz		w3, .Lmacout
 	ld1		{v1.16b}, [x0], #16		/* get next pt block */
 	eor		v0.16b, v0.16b, v1.16b		/* ..and xor with dg */
 
 	subs		w3, w3, #1
-	csinv		x5, x6, xzr, eq
+	csinv		w5, w6, wzr, eq
 	cbz		w5, .Lmacout
 
 .Lmacenc:
 	encrypt_block	v0, w2, x1, x7, w8
 	b		.Lmacloop
-- 
2.53.0

Re: [PATCH 03/15] crypto: arm64/aes - Fix 32-bit aes_mac_update() arg treated as 64-bit

[Ard Biesheuvel]

On Wed, 18 Feb 2026, at 22:34, Eric Biggers wrote:
> Since the 'enc_after' argument to neon_aes_mac_update() and
> ce_aes_mac_update() has type 'int', it needs to be accessed using the
> corresponding 32-bit register, not the 64-bit register.  The upper half
> of the corresponding 64-bit register may contain garbage.
>

How could that happen? Setting the 32-bit alias of a GPR clears the upper half.

> Fixes: 4860620da7e5 ("crypto: arm64/aes - add NEON/Crypto Extensions 
> CBCMAC/CMAC/XCBC driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>

Agree with the change but I don't think this needs a cc:stable (or a fixes tag)

> ---
>  arch/arm64/crypto/aes-modes.S | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/crypto/aes-modes.S b/arch/arm64/crypto/aes-modes.S
> index 0e834a2c062c..e793478f37c1 100644
> --- a/arch/arm64/crypto/aes-modes.S
> +++ b/arch/arm64/crypto/aes-modes.S
> @@ -836,11 +836,11 @@ AES_FUNC_START(aes_mac_update)
>  	encrypt_block	v0, w2, x1, x7, w8
>  	eor		v0.16b, v0.16b, v3.16b
>  	encrypt_block	v0, w2, x1, x7, w8
>  	eor		v0.16b, v0.16b, v4.16b
>  	cmp		w3, wzr
> -	csinv		x5, x6, xzr, eq
> +	csinv		w5, w6, wzr, eq
>  	cbz		w5, .Lmacout
>  	encrypt_block	v0, w2, x1, x7, w8
>  	st1		{v0.16b}, [x4]			/* return dg */
>  	cond_yield	.Lmacout, x7, x8
>  	b		.Lmacloop4x
> @@ -850,11 +850,11 @@ AES_FUNC_START(aes_mac_update)
>  	cbz		w3, .Lmacout
>  	ld1		{v1.16b}, [x0], #16		/* get next pt block */
>  	eor		v0.16b, v0.16b, v1.16b		/* ..and xor with dg */
> 
>  	subs		w3, w3, #1
> -	csinv		x5, x6, xzr, eq
> +	csinv		w5, w6, wzr, eq
>  	cbz		w5, .Lmacout
> 
>  .Lmacenc:
>  	encrypt_block	v0, w2, x1, x7, w8
>  	b		.Lmacloop
> -- 
> 2.53.0

Re: [PATCH 03/15] crypto: arm64/aes - Fix 32-bit aes_mac_update() arg treated as 64-bit

[Eric Biggers]

On Thu, Feb 19, 2026 at 10:23:39AM +0100, Ard Biesheuvel wrote:
> On Wed, 18 Feb 2026, at 22:34, Eric Biggers wrote:
> > Since the 'enc_after' argument to neon_aes_mac_update() and
> > ce_aes_mac_update() has type 'int', it needs to be accessed using the
> > corresponding 32-bit register, not the 64-bit register.  The upper half
> > of the corresponding 64-bit register may contain garbage.
> 
> How could that happen? Setting the 32-bit alias of a GPR clears the upper half.

The ABI doesn't guarantee that the upper 32 bits are cleared.  Try the
following:

void g(unsigned int a);

void f(unsigned long long a)
{
	g((unsigned int)a);
}

Both gcc and clang generate code that simply tail-calls g(), leaving the
upper 32 bits unchanged rather than zeroing them as per the cast:

0000000000000000 <f>:
       0: 14000000     	b	0x0 <f>

So it's possible.  Now, it's certainly unlikely to happen in practice,
as the real code doesn't use truncating casts like that, and the
instructions that write to the 32-bit registers clear the upper 64 bits
-- as you noted and as I've noted before in similar fixes (e.g.
https://lore.kernel.org/r/20251102234209.62133-2-ebiggers@kernel.org/).

So does it really matter?  Probably not.  However, given that the
correct behavior wasn't *guaranteed*, I think that to be safe we should
continue to consider patches like this to be bugfixes.

- Eric