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

[Thomas Huth <thuth@redhat.com>]

On 14/07/2026 01.54, Eric Biggers wrote:
> 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.
Ok, but then maybe add a comment here. Otherwise people will wonder why it 
has been done like this when reading the code later.

FWIW, I doubt that this really makes a big difference here. Looking at a 
function that contains your code, the disassembly currently looks like this 
(with -O2):

0000000000000000 <inc_be128_ctr>:
    0:   80 47 0f 01             addb   $0x1,0xf(%rdi)
    4:   48 8d 57 0e             lea    0xe(%rdi),%rdx
    8:   74 0a                   je     14 <inc_be128_ctr+0x14>
    a:   c3                      ret
    b:   0f 1f 44 00 00          nopl   0x0(%rax,%rax,1)
   10:   48 83 ea 01             sub    $0x1,%rdx
   14:   80 02 01                addb   $0x1,(%rdx)
   17:   75 05                   jne    1e <inc_be128_ctr+0x1e>
   19:   48 39 d7                cmp    %rdx,%rdi
   1c:   75 f2                   jne    10 <inc_be128_ctr+0x10>
   1e:   c3                      ret

So that's 3 assembly instructions 'til you reach the "ret".

When you drop the optimization, it looks like this:

0000000000000000 <inc_be128_ctr>:
    0:   48 8d 57 0f             lea    0xf(%rdi),%rdx
    4:   eb 0e                   jmp    14 <inc_be128_ctr+0x14>
    6:   66 2e 0f 1f 84 00 00    cs nopw 0x0(%rax,%rax,1)
    d:   00 00 00
   10:   48 83 ea 01             sub    $0x1,%rdx
   14:   80 02 01                addb   $0x1,(%rdx)
   17:   75 05                   jne    1e <inc_be128_ctr+0x1e>
   19:   48 39 d7                cmp    %rdx,%rdi
   1c:   75 f2                   jne    10 <inc_be128_ctr+0x10>
   1e:   c3                      ret

That's 4 assembly instructions 'til you reach the "ret". Not such a big 
difference...?

And with -O3, both variants end up with the same code.

  Thomas