linux-kernel - Re: [PATCH v2 3/3] padata: avoid UAF for reorder

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <faa23d11-4387-4952-bd29-034b4558668c@huaweicloud.com>
Date: Mon, 26 May 2025 09:15:37 +0800
From: Chen Ridong <chenridong@...weicloud.com>
To: Herbert Xu <herbert@...dor.apana.org.au>
Cc: steffen.klassert@...unet.com, daniel.m.jordan@...cle.com,
 nstange@...e.de, linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org,
 chenridong@...wei.com, wangweiyang2@...wei.com
Subject: Re: [PATCH v2 3/3] padata: avoid UAF for reorder_work



On 2025/5/23 11:59, Herbert Xu wrote:
> On Fri, Jan 10, 2025 at 06:16:39AM +0000, Chen Ridong wrote:
>> From: Chen Ridong <chenridong@...wei.com>
>>
>> Although the previous patch can avoid ps and ps UAF for _do_serial, it
>> can not avoid potential UAF issue for reorder_work. This issue can
>> happen just as below:
>>
>> crypto_request			crypto_request		crypto_del_alg
>> padata_do_serial
>>   ...
>>   padata_reorder
>>     // processes all remaining
>>     // requests then breaks
>>     while (1) {
>>       if (!padata)
>>         break;
>>       ...
>>     }
>>
>> 				padata_do_serial
>> 				  // new request added
>> 				  list_add
>>     // sees the new request
>>     queue_work(reorder_work)
>> 				  padata_reorder
>> 				    queue_work_on(squeue->work)
>> ...
>>
>> 				<kworker context>
>> 				padata_serial_worker
>> 				// completes new request,
>> 				// no more outstanding
>> 				// requests
>>
>> 							crypto_del_alg
>> 							  // free pd
>>
>> <kworker context>
>> invoke_padata_reorder
>>   // UAF of pd
> 
> Looking back this explanation is actually broken.  The call
> crypto_del_alg does not free anything immediately.  It can only
> start freeing things once the final tfm user goes away.  Any crypto
> request of that tfm must have completed before that happens.
> 
> If not there is a serious bug in the Crypto API.
> 
> So if crypto_del_alg is leading to a freeing of the pd while there
> are still outstanding users of that tfm, then this points to a bug
> in the Crypto API and not padata.
> 
> Can you still reproduce this bug easily if you revert the patches
> in this series? If so we should be able to track down the real bug.
> 

Unfortunately, I did not reproduce this bug, It was mentioned:
https://lore.kernel.org/all/20221019083708.27138-6-nstange@suse.de/

We through that the kworker is asynchronous, and this scenarios may happen.

Thanks,
Ridong

> To recap, every tfm holds a ref count on the underlying crypto_alg.
> All crypto requests must complete before a tfm can be freed, which
> then leads to a drop of the refcount on crypto_alg.
> 
> A crypto_alg can only be freed when its ref count hits zero.  Only
> then will the associated pd be freed.
> 
> So what's missing in the above picture is the entity that is freeing
> the tfm, thus leading to the actual freeing of the alg and pd.
> 
> Thanks,