[PATCH v2] crypto: x86/aes-gcm - fix always true check for last AAD segment

[PATCH v2] crypto: x86/aes-gcm - fix always true check for last AAD segment

[Mohamad Raizudeen]

In gcm_process_assoc(), a segment that is not the last one must have its
length rounded down to a multiple of 16 bytes, as required by the
assembly. The check for a non-last segment is `if (unlikely(assoclen))
/* Not the last segment yet? */` where assoclen is the number of AAD
bytes remaining after the current segment.

Since the conversion to the new scatterwalk API, assoclen is decremented
at the end of the loop body rather than the beginning, so it still
includes the current segment when the check executes, making the check
always true.

As a result the last segment was rounded down as well, causing some
avoidable extra work: an additional memcpy into the temporary buffer and
an additional call into the assembly afte the loop. The GCM
authentication tag is unaffected either way, so the self-tests pass and
this went unnoticed. It is purely an efficiency issue rather than a
correctness one.

Fix this by moving the assoclen decrement back to the beginning of the
loop body.

Fixes: e9787deff49ea ("crypto: x86/aes-gcm - use the new scatterwalk functions")
Cc: stable@vger.kernel.org
Suggested-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Mohamad Raizudeen <raizudeen.kerneldev@gmail.com>
---
Changes in v2:
- Use the correct Fixes commit and cc to stable
- Change the fix by moving the assoclen decrement back to the top of the
  loop body instead of changing the condition as suggested by Eric
  Biggers.

Link to v1: https://lore.kernel.org/all/20260926040614.8683-1-raizudeen.kerneldev@gmail.com/T/

 arch/x86/crypto/aesni-intel_glue.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/crypto/aesni-intel_glue.c b/arch/x86/crypto/aesni-intel_glue.c
index f522fff9231e..1903ed8bbe3b 100644
--- a/arch/x86/crypto/aesni-intel_glue.c
+++ b/arch/x86/crypto/aesni-intel_glue.c
@@ -1293,6 +1293,7 @@ static void gcm_process_assoc(const struct aes_gcm_key *key, u8 ghash_acc[16],
 		unsigned int len;
 		const u8 *src = walk.addr;
 
+		assoclen -= orig_len_this_step;
 		if (unlikely(pos)) {
 			len = min(len_this_step, 16 - pos);
 			memcpy(&buf[pos], src, len);
@@ -1320,7 +1321,6 @@ static void gcm_process_assoc(const struct aes_gcm_key *key, u8 ghash_acc[16],
 			kernel_fpu_end();
 			kernel_fpu_begin();
 		}
-		assoclen -= orig_len_this_step;
 	}
 	if (unlikely(pos))
 		aes_gcm_aad_update(key, ghash_acc, buf, pos, flags);
-- 
2.53.0

Re: [PATCH v2] crypto: x86/aes-gcm - fix always true check for last AAD segment

[Eric Biggers]

On Sat, Sep 26, 2026 at 08:29:03PM +0530, Mohamad Raizudeen wrote:
> In gcm_process_assoc(), a segment that is not the last one must have its
> length rounded down to a multiple of 16 bytes, as required by the
> assembly. The check for a non-last segment is `if (unlikely(assoclen))
> /* Not the last segment yet? */` where assoclen is the number of AAD
> bytes remaining after the current segment.
> 
> Since the conversion to the new scatterwalk API, assoclen is decremented
> at the end of the loop body rather than the beginning, so it still
> includes the current segment when the check executes, making the check
> always true.
> 
> As a result the last segment was rounded down as well, causing some
> avoidable extra work: an additional memcpy into the temporary buffer and
> an additional call into the assembly afte the loop. The GCM
> authentication tag is unaffected either way, so the self-tests pass and
> this went unnoticed. It is purely an efficiency issue rather than a
> correctness one.
> 
> Fix this by moving the assoclen decrement back to the beginning of the
> loop body.
> 
> Fixes: e9787deff49ea ("crypto: x86/aes-gcm - use the new scatterwalk functions")
> Cc: stable@vger.kernel.org
> Suggested-by: Eric Biggers <ebiggers@kernel.org>
> Signed-off-by: Mohamad Raizudeen <raizudeen.kerneldev@gmail.com>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git/log/?h=libcrypto-fixes

Thanks!

- Eric