Re: [PATCH v5 2/4] crypto: ti - Add support for AES-CTR in DTHEv2 driver

[T Pratham <t-pratham@ti.com>]

Hi Herbert,
Thanks for review.
On 31/10/25 15:06, Herbert Xu wrote:
> On Wed, Oct 22, 2025 at 11:15:40PM +0530, T Pratham wrote:
>>
>> +	if (ctx->aes_mode == DTHE_AES_CTR) {
>> +		/*
>> +		 * CTR mode can operate on any input length, but the hardware
>> +		 * requires input length to be a multiple of the block size.
>> +		 * We need to handle the padding in the driver.
>> +		 */
>> +		if (req->cryptlen % AES_BLOCK_SIZE) {
>> +			/* Need to create a new SG list with padding */
>> +			pad_len = ALIGN(req->cryptlen, AES_BLOCK_SIZE) - req->cryptlen;
>> +			struct scatterlist *sg;
>> +
>> +			src = kmalloc_array((src_nents + 1), sizeof(*src), GFP_KERNEL);
> 
> You can't allocate memory on the data path.  The request might have
> been issued by the storage layer and doing a GFP_KERNEL allocation
> here risks dead-lock.
> 
> Failing the allocation is also not good.
> 
> Ideally you should make the hardware deal with the multiple of block
> size data, and then handle the trailer in your driver.
> 
> But if it's too hard just send the whole thing to the fallback.
> 

Understood.
Ideally, I'd like to avoid fallback as much as possible. While it does
sound lucrative to handle the trailer in s/w and let h/w handle blocks,
I have another proposal to avoid memory allocations here: using
scatterlist chaining.

Padding:

struct scatterlist src_pad[2];
struct scatterlist *sg;

sg_init_table(src_pad, 2);
sg = sg_last(req->src, src_nents);
sg_set_page(&src_pad[0], sg_page(sg), sg->length, sg->offset);
sg_unmark_end(&src_pad[0]);
sg_set_buf(&src_pad[1], pad_buf, pad_len);

if (src_nents == 1)
	src = src_pad;
else
	sg_chain(sg, 1, src_pad);

src_nents++;


Cleanup (restoring original req->src's last nent):

if (src_nents > 2) {
	struct scatterlist *sg;
	unsigned int i;

	for (i = 0, sg = req->src; i < src_nents - 3; ++i)
		sg = sg_next(sg);
	sg++;
	sg->page_link &= ~SG_CHAIN;
	sg_set_page(sg, sg_page(&src_pad[0]),
		    src_pad[0].length, src_pad[0].offset);
	sg_mark_end(sg);
}


Let me know if this is fine.

However, I suppose we similarly cannot do allocations in AEADs as well?
If that is the case, my current code relies a lot on it (using sg_split
to separate AAD from {plain, cipher}text and applying padding like CTR
here, both of which allocate memory).

I can change padding to avoid kmallocs like CTR easily. But separating
AAD and plaintext/ciphertext without using sg_split will be a task. Let
me know if this is correct, so that I'll split the series and send
ciphers now, and AEADs later.

-- 
Regards
T Pratham <t-pratham@ti.com>