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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <f4c339e3-0964-469e-9cec-eeaf2e2f078b@ti.com>
Date: Fri, 31 Oct 2025 20:11:34 +0530
From: T Pratham <t-pratham@...com>
To: Herbert Xu <herbert@...dor.apana.org.au>
CC: "David S. Miller" <davem@...emloft.net>, Manorit Chawdhry
	<m-chawdhry@...com>, Kamlesh Gurudasani <kamlesh@...com>, Shiva Tripathi
	<s-tripathi1@...com>, Kavitha Malarvizhi <k-malarvizhi@...com>, "Vishal
 Mahaveer" <vishalm@...com>, Praneeth Bajjuri <praneeth@...com>,
	<linux-crypto@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 2/4] crypto: ti - Add support for AES-CTR in DTHEv2
 driver

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@...com>