diff mbox series

padata: Fix refcnt handling in padata_free_shell() again

Message ID 20241203153426.62794-1-zachwade.k@gmail.com
State New
Headers show
Series padata: Fix refcnt handling in padata_free_shell() again | expand

Commit Message

Zach Wade Dec. 3, 2024, 3:34 p.m. UTC 
testcases/kernel/crypto/pcrypt_aead01.c of LTP project has UAF.

Steps to reproduce:
1. /sys/module/cryptomgr/parameters/notests is N
2. run LTP ./testcases/bin/pcrypt_aead01

There is a race condition when padata_free_shell is released, which
causes it to be accessed after being released. We should use the rcu
mechanism to protect it.
            cpu0                |               cpu1
================================================================
padata_do_parallel              |   padata_free_shell
    rcu_read_lock_bh            |       refcount_dec_and_test
    # run to here <- 1          |       # run to here <- 2
    refcount_inc(&pd->refcnt);  |           padata_free_pd <- 3
    padata_work_alloc		|	...
    rcu_read_unlock_bh          |
				|
There is a possibility of UAF after refcount_inc(&pd->refcnt).

kasan report:
[158753.658839] ==================================================================
[158753.658851] BUG: KASAN: slab-use-after-free in padata_find_next+0x2d6/0x3f0
[158753.658868] Read of size 4 at addr ffff88812f8b8524 by task kworker/u158:0/988818
[158753.658878]
[158753.658885] CPU: 23 UID: 0 PID: 988818 Comm: kworker/u158:0 Kdump: loaded Tainted: G        W   E      6.12.0-dirty #33
[158753.658902] Tainted: [W]=WARN, [E]=UNSIGNED_MODULE
[158753.658907] Hardware name: VMware, Inc. VMware20,1/440BX Desktop Reference Platform, BIOS VMW201.00V.20192059.B64.2207280713 07/28/2022
[158753.658914] Workqueue: pdecrypt_parallel padata_parallel_worker
[158753.658927] Call Trace:
[158753.658932]  <TASK>
[158753.658938]  dump_stack_lvl+0x5d/0x80
[158753.658960]  print_report+0x174/0x505
[158753.658992]  kasan_report+0xe0/0x160
[158753.659013]  padata_find_next+0x2d6/0x3f0
[158753.659035]  padata_reorder+0x1cc/0x400
[158753.659043]  padata_parallel_worker+0x70/0x160
[158753.659051]  process_one_work+0x646/0xeb0
[158753.659061]  worker_thread+0x619/0x10e0
[158753.659092]  kthread+0x28d/0x350
[158753.659102]  ret_from_fork+0x31/0x70
[158753.659111]  ret_from_fork_asm+0x1a/0x30
[158753.659117]  </TASK>
[158753.659119]
[158753.659120] Allocated by task 1027931:
[158753.659123]  kasan_save_stack+0x30/0x50
[158753.659126]  kasan_save_track+0x14/0x30
[158753.659128]  __kasan_kmalloc+0xaa/0xb0
[158753.659130]  padata_alloc_pd+0x69/0x9f0
[158753.659132]  padata_alloc_shell+0x82/0x210
[158753.659134]  pcrypt_create+0x13b/0x7a0 [pcrypt]
[158753.659139]  cryptomgr_probe+0x8d/0x230
[158753.659144]  kthread+0x28d/0x350
[158753.659147]  ret_from_fork+0x31/0x70
[158753.659150]  ret_from_fork_asm+0x1a/0x30
[158753.659152]
[158753.659153] Freed by task 1024357:
[158753.659155]  kasan_save_stack+0x30/0x50
[158753.659158]  kasan_save_track+0x14/0x30
[158753.659160]  kasan_save_free_info+0x3b/0x70
[158753.659164]  __kasan_slab_free+0x4f/0x70
[158753.659167]  kfree+0x119/0x440
[158753.659172]  padata_free_shell+0x262/0x320
[158753.659174]  pcrypt_free+0x43/0x90 [pcrypt]
[158753.659177]  crypto_destroy_instance_workfn+0x79/0xc0
[158753.659182]  process_one_work+0x646/0xeb0
[158753.659184]  worker_thread+0x619/0x10e0
[158753.659186]  kthread+0x28d/0x350
[158753.659188]  ret_from_fork+0x31/0x70
[158753.659191]  ret_from_fork_asm+0x1a/0x30
[158753.659194]
[158753.659195] The buggy address belongs to the object at ffff88812f8b8500
 which belongs to the cache kmalloc-192 of size 192
[158753.659198] The buggy address is located 36 bytes inside of
 freed 192-byte region [ffff88812f8b8500, ffff88812f8b85c0)
[158753.659202]
[158753.659203] The buggy address belongs to the physical page:
[158753.659205] page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x12f8b8
[158753.659209] head: order:1 mapcount:0 entire_mapcount:0 nr_pages_mapped:0 pincount:0
[158753.659212] anon flags: 0x10000000000040(head|node=0|zone=2)
[158753.659216] page_type: f5(slab)
[158753.659220] raw: 0010000000000040 ffff88810004c3c0 0000000000000000 dead000000000001
[158753.659223] raw: 0000000000000000 0000000080200020 00000001f5000000 0000000000000000
[158753.659225] head: 0010000000000040 ffff88810004c3c0 0000000000000000 dead000000000001
[158753.659228] head: 0000000000000000 0000000080200020 00000001f5000000 0000000000000000
[158753.659230] head: 0010000000000001 ffffea0004be2e01 ffffffffffffffff 0000000000000000
[158753.659232] head: ffff888100000002 0000000000000000 00000000ffffffff 0000000000000000
[158753.659234] page dumped because: kasan: bad access detected
[158753.659235]
[158753.659236] Memory state around the buggy address:
[158753.659238]  ffff88812f8b8400: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[158753.659240]  ffff88812f8b8480: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
[158753.659242] >ffff88812f8b8500: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[158753.659243]                                ^
[158753.659245]  ffff88812f8b8580: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
[158753.659247]  ffff88812f8b8600: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[158753.659248] ==================================================================

Fixes: 07928d9bfc81 ("padata: Remove broken queue flushing")

Co-developed-by: Ding Hui <dinghui@sangfor.com.cn>
Signed-off-by: Ding Hui <dinghui@sangfor.com.cn>
Signed-off-by: Zach Wade <zachwade.k@gmail.com>
---
 include/linux/padata.h |  1 +
 kernel/padata.c        | 11 +++++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

Comments

Daniel Jordan Dec. 5, 2024, 11:17 p.m. UTC | #1 
Hello Zach,

On Tue, Dec 03, 2024 at 11:34:26PM +0800, Zach Wade wrote:
> testcases/kernel/crypto/pcrypt_aead01.c of LTP project has UAF.
> 
> Steps to reproduce:
> 1. /sys/module/cryptomgr/parameters/notests is N
> 2. run LTP ./testcases/bin/pcrypt_aead01
> 
> There is a race condition when padata_free_shell is released, which
> causes it to be accessed after being released. We should use the rcu
> mechanism to protect it.
>             cpu0                |               cpu1
> ================================================================
> padata_do_parallel              |   padata_free_shell
>     rcu_read_lock_bh            |       refcount_dec_and_test
>     # run to here <- 1          |       # run to here <- 2
>     refcount_inc(&pd->refcnt);  |           padata_free_pd <- 3
>     padata_work_alloc		|	...
>     rcu_read_unlock_bh          |

I'm not sure this scenario is possible.  For padata_free_shell to be
called, I think there can't be any allocated pcrypt aead transforms (so
that the alg refcount is low enough for crypto_del_alg to call
padata_free_shell), but here cpu0 has called padata_do_parallel which
implies it has such a transform.

> There is a possibility of UAF after refcount_inc(&pd->refcnt).
> 
> kasan report:
> [158753.658839] ==================================================================
> [158753.658851] BUG: KASAN: slab-use-after-free in padata_find_next+0x2d6/0x3f0
> [158753.658868] Read of size 4 at addr ffff88812f8b8524 by task kworker/u158:0/988818
> [158753.658878]
> [158753.658885] CPU: 23 UID: 0 PID: 988818 Comm: kworker/u158:0 Kdump: loaded Tainted: G        W   E      6.12.0-dirty #33
> [158753.658902] Tainted: [W]=WARN, [E]=UNSIGNED_MODULE
> [158753.658907] Hardware name: VMware, Inc. VMware20,1/440BX Desktop Reference Platform, BIOS VMW201.00V.20192059.B64.2207280713 07/28/2022
> [158753.658914] Workqueue: pdecrypt_parallel padata_parallel_worker
> [158753.658927] Call Trace:
> [158753.658932]  <TASK>
> [158753.658938]  dump_stack_lvl+0x5d/0x80
> [158753.658960]  print_report+0x174/0x505
> [158753.658992]  kasan_report+0xe0/0x160
> [158753.659013]  padata_find_next+0x2d6/0x3f0
> [158753.659035]  padata_reorder+0x1cc/0x400
> [158753.659043]  padata_parallel_worker+0x70/0x160

This report shows the uaf in find_next, not do_parallel.  I think this
is the same issue reported recently in this series:

    https://lore.kernel.org/all/20241123080509.2573987-1-chenridong@huaweicloud.com/
diff mbox series

Patch

diff --git a/include/linux/padata.h b/include/linux/padata.h
index 0146daf34430..ee6155689e47 100644
--- a/include/linux/padata.h
+++ b/include/linux/padata.h
@@ -103,6 +103,7 @@  struct parallel_data {
 	int				cpu;
 	struct padata_cpumask		cpumask;
 	struct work_struct		reorder_work;
+	struct rcu_head			rcu;
 	spinlock_t                      ____cacheline_aligned lock;
 };
 
diff --git a/kernel/padata.c b/kernel/padata.c
index d51bbc76b227..3afdccc7e20e 100644
--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -1109,6 +1109,14 @@  struct padata_shell *padata_alloc_shell(struct padata_instance *pinst)
 }
 EXPORT_SYMBOL(padata_alloc_shell);
 
+static void __padata_put_pd(struct rcu_head *head)
+{
+	struct parallel_data *pd = container_of(head, struct parallel_data, rcu);
+
+	if (refcount_dec_and_test(&pd->refcnt))
+		padata_free_pd(pd);
+}
+
 /**
  * padata_free_shell - free a padata shell
  *
@@ -1124,9 +1132,8 @@  void padata_free_shell(struct padata_shell *ps)
 	mutex_lock(&ps->pinst->lock);
 	list_del(&ps->list);
 	pd = rcu_dereference_protected(ps->pd, 1);
-	if (refcount_dec_and_test(&pd->refcnt))
-		padata_free_pd(pd);
 	mutex_unlock(&ps->pinst->lock);
+	call_rcu(&pd->rcu, __padata_put_pd);
 
 	kfree(ps);
 }