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: <aC_yoWXJcsLxfLR4@gondor.apana.org.au>
Date: Fri, 23 May 2025 11:59:29 +0800
From: Herbert Xu <herbert@...dor.apana.org.au>
To: Chen Ridong <chenridong@...weicloud.com>
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 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.

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,
-- 
Email: Herbert Xu <herbert@...dor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt