diff mbox series

[v2,3/3] padata: avoid UAF for reorder_work

Message ID 20250110061639.1280907-4-chenridong@huaweicloud.com
State New
Headers show
Series padata: fix UAF issues | expand

Commit Message

Chen Ridong Jan. 10, 2025, 6:16 a.m. UTC 
From: Chen Ridong <chenridong@huawei.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

To avoid UAF for 'reorder_work', get 'pd' ref before put 'reorder_work'
into the 'serial_wq' and put 'pd' ref until the 'serial_wq' finish.

Fixes: bbefa1dd6a6d ("crypto: pcrypt - Avoid deadlock by using per-instance padata queues")
Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
 kernel/padata.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Daniel Jordan Jan. 13, 2025, 5 p.m. UTC | #1 
On Fri, Jan 10, 2025 at 06:16:39AM +0000, Chen Ridong wrote:
...
> Fixes: bbefa1dd6a6d ("crypto: pcrypt - Avoid deadlock by using per-instance padata queues")
> Signed-off-by: Chen Ridong <chenridong@huawei.com>

Series looks good, thanks for the persistence.

Acked-by: Daniel Jordan <daniel.m.jordan@oracle.com>

> diff --git a/kernel/padata.c b/kernel/padata.c
...
>  static void invoke_padata_reorder(struct work_struct *work)
> @@ -364,6 +370,8 @@ static void invoke_padata_reorder(struct work_struct *work)
>  	pd = container_of(work, struct parallel_data, reorder_work);
>  	padata_reorder(pd);
>  	local_bh_enable();
> +	/* Pairs with putting the reorder_work in the serial_wq */

s/putting/getting/
chenridong Jan. 14, 2025, 12:22 p.m. UTC | #2 
On 2025/1/14 1:00, Daniel Jordan wrote:
> On Fri, Jan 10, 2025 at 06:16:39AM +0000, Chen Ridong wrote:
> ...
>> Fixes: bbefa1dd6a6d ("crypto: pcrypt - Avoid deadlock by using per-instance padata queues")
>> Signed-off-by: Chen Ridong <chenridong@huawei.com>
> 
> Series looks good, thanks for the persistence.
> 

Thank you for your patience.

Best regards,
Ridong
> Acked-by: Daniel Jordan <daniel.m.jordan@oracle.com>
> 
>> diff --git a/kernel/padata.c b/kernel/padata.c
> ...
>>  static void invoke_padata_reorder(struct work_struct *work)
>> @@ -364,6 +370,8 @@ static void invoke_padata_reorder(struct work_struct *work)
>>  	pd = container_of(work, struct parallel_data, reorder_work);
>>  	padata_reorder(pd);
>>  	local_bh_enable();
>> +	/* Pairs with putting the reorder_work in the serial_wq */
> 
> s/putting/getting/
>
diff mbox series

Patch

diff --git a/kernel/padata.c b/kernel/padata.c
index de2c02a81469..418987056340 100644
--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -352,8 +352,14 @@  static void padata_reorder(struct parallel_data *pd)
 	smp_mb();
 
 	reorder = per_cpu_ptr(pd->reorder_list, pd->cpu);
-	if (!list_empty(&reorder->list) && padata_find_next(pd, false))
+	if (!list_empty(&reorder->list) && padata_find_next(pd, false)) {
+		/*
+		 * Other context(eg. the padata_serial_worker) can finish the request.
+		 * To avoid UAF issue, add pd ref here, and put pd ref after reorder_work finish.
+		 */
+		padata_get_pd(pd);
 		queue_work(pinst->serial_wq, &pd->reorder_work);
+	}
 }
 
 static void invoke_padata_reorder(struct work_struct *work)
@@ -364,6 +370,8 @@  static void invoke_padata_reorder(struct work_struct *work)
 	pd = container_of(work, struct parallel_data, reorder_work);
 	padata_reorder(pd);
 	local_bh_enable();
+	/* Pairs with putting the reorder_work in the serial_wq */
+	padata_put_pd(pd);
 }
 
 static void padata_serial_worker(struct work_struct *serial_work)