Re: [PATCH 04/33] lib/crypto: aes: Add CTR and XCTR support

[Eric Biggers]

On Mon, Jul 13, 2026 at 10:39:53AM +0200, Thomas Huth wrote:
> > diff --git a/Documentation/crypto/libcrypto-unauth-encryption.rst b/Documentation/crypto/libcrypto-unauth-encryption.rst
> > index fb8106034089..6aca01d715da 100644
> > --- a/Documentation/crypto/libcrypto-unauth-encryption.rst
> > +++ b/Documentation/crypto/libcrypto-unauth-encryption.rst
> > @@ -27,6 +27,13 @@ Support for AES in the CBC and CBC-CTS modes of operation.
> >   .. kernel-doc:: include/crypto/aes-cbc.h
> > +AES-CTR and AES-XCTR
> > +--------------------
> > +
> > +Support for AES in the CTR and XCTR modes of operation.
> 
> I guess you already have this on your radar, but just in case: It would be
> nice to turn this into a full sentence, too.

Yes, I'm making all of them full sentences.

> > +/**
> > + * aes_ctr() - AES-CTR en/decryption
> > + * @dst: The destination buffer.  Can be in-place or out-of-place.  For other
> > + *	 overlaps the behavior is unspecified.
> > + * @src: The source data
> > + * @len: Number of bytes to en/decrypt
> > + * @ctr: The counter.  It will be incremented by ceil(@len / AES_BLOCK_SIZE).
> > + * @key: The key
> > + *
> > + * This implements AES in counter mode with a 128-bit big endian counter.
> > + *
> > + * This supports incremental en/decryption.  The length of each non-final chunk
> > + * must be a multiple of AES_BLOCK_SIZE, and the updated @ctr must be passed in
> > + * each time.
> 
> Maybe add some wording that ctr ideally should not be 0 for the first call,
> i.e. a "nonce" value?

It depends on the usage.  If a distinct key is used for each message for
example, always starting at 0 is perfectly fine.

I'm not sure how far we should go to document the proper use of each
algorithm.  Really the AES-CTR support is just for internal use by
AES-GCM and AES-CCM, and a few odd users that implement specific other
protocols that need AES-CTR.  It's not intended to be a place to go to
receive an introduction to CTR mode.

> > +static __always_inline void inc_be128_ctr(u8 ctr[AES_BLOCK_SIZE])
> > +{
> > +	/* Casts to u8 are needed because of the implicit integer promotion. */
> > +	if (((u8)++ctr[AES_BLOCK_SIZE - 1]) != 0)
> > +		return;
> 
> Why do you handle the first value separately here? The code could be
> simplified to start with "int i = AES_BLOCK_SIZE -1" in the for-loop
> instead?

Just a trick to optimize performance by unrolling the first iteration,
since 255 times out of 256 the first iteration is enough.

> > +void aes_xctr(u8 *dst, const u8 *src, size_t len, u64 *ctr,
> > +	      const u8 iv[AES_BLOCK_SIZE], aes_encrypt_arg key)
> > +{
> > +	const __le64 iv0 = get_unaligned((const __le64 *)&iv[0]);
> > +	__le64 aes_input[2];
> > +	u8 keystream[AES_BLOCK_SIZE] __aligned(__alignof__(long));
> > +
> > +	if (likely(aes_xctr_arch(dst, src, len, ctr, iv, key.enc_key)))
> > +		return;
> > +
> > +	aes_input[1] = get_unaligned((const __le64 *)&iv[8]);
> > +	/* Handle the full blocks. */
> > +	for (; len >= AES_BLOCK_SIZE; len -= AES_BLOCK_SIZE) {
> > +		aes_input[0] = iv0 ^ cpu_to_le64((*ctr)++);
> 
> Do we want to have a BUG_ON or WARN_ON_ONCE somewhere to check that ctr does
> not wrap around (i.e. to make sure that ctr was really 1 for the first
> call)? Something like:
> 
> 	WARN_ON_ONCE((s64)(cpu_to_le64(*ctr) + len / AES_BLOCK_SIZE) < 0)
> 
> at the beginning of the function?

Maybe.  Since the counter is a u64, and this function isn't going to be
called from very many places, I don't think it would be a particularly
valuable WARN_ON_ONCE.  It shouldn't be BUG_ON.

- Eric