Skip to contentSkip to conversation

erofs: Fix detection of atomic context

PATCH Applied Artem Dinaburg 3 participants 6 messages Sep 22 – Sep 25

Subsystems (2)
Stable branch Arm MediaTek
lore
Share & tools
lore

Artem Dinaburg

author
#0 lore raw

PATCH 1/1 erofs: Fix detection of atomic context

From: Sandeep Dhavale <[email protected]>

[ Upstream commit 12d0a24afd9ea58e581ea64d64e066f2027b28d9 ]

Current check for atomic context is not sufficient as z_erofs_decompressqueue_endio can be called under rcu lock from blk_mq_flush_plug_list(). See the stacktrace [1]

In such case we should hand off the decompression work for async processing rather than trying to do sync decompression in current context. Patch fixes the detection by checking for rcu_read_lock_any_held() and while at it use more appropriate !in_task() check than in_atomic().

Background: Historically erofs would always schedule a kworker for decompression which would incur the scheduling cost regardless of the context. But z_erofs_decompressqueue_endio() may not always be in atomic context and we could actually benefit from doing the decompression in z_erofs_decompressqueue_endio() if we are in thread context, for example when running with dm-verity. This optimization was later added in patch [2] which has shown improvement in performance benchmarks.

==============================================
[1] Problem stacktrace
[name:core&]BUG: sleeping function called from invalid context at kernel/locking/mutex.c:291 [name:core&]in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 1615, name: CpuMonitorServi [name:core&]preempt_count: 0, expected: 0
[name:core&]RCU nest depth: 1, expected: 0

CPU: 7 PID: 1615 Comm: CpuMonitorServi Tainted: G S      W  OE      6.1.25-android14-5-maybe-dirty-mainline #1
Hardware name: MT6897 (DT)
Call trace:
 dump_backtrace+0x108/0x15c
 show_stack+0x20/0x30
 dump_stack_lvl+0x6c/0x8c
 dump_stack+0x20/0x48
 __might_resched+0x1fc/0x308
 __might_sleep+0x50/0x88
 mutex_lock+0x2c/0x110
 z_erofs_decompress_queue+0x11c/0xc10
 z_erofs_decompress_kickoff+0x110/0x1a4
 z_erofs_decompressqueue_endio+0x154/0x180
 bio_endio+0x1b0/0x1d8
 __dm_io_complete+0x22c/0x280
 clone_endio+0xe4/0x280
 bio_endio+0x1b0/0x1d8
 blk_update_request+0x138/0x3a4
 blk_mq_plug_issue_direct+0xd4/0x19c
 blk_mq_flush_plug_list+0x2b0/0x354
 __blk_flush_plug+0x110/0x160
 blk_finish_plug+0x30/0x4c
 read_pages+0x2fc/0x370
 page_cache_ra_unbounded+0xa4/0x23c
 page_cache_ra_order+0x290/0x320
 do_sync_mmap_readahead+0x108/0x2c0
 filemap_fault+0x19c/0x52c
 __do_fault+0xc4/0x114
 handle_mm_fault+0x5b4/0x1168
 do_page_fault+0x338/0x4b4
 do_translation_fault+0x40/0x60
 do_mem_abort+0x60/0xc8
 el0_da+0x4c/0xe0
 el0t_64_sync_handler+0xd4/0xfc
 el0t_64_sync+0x1a0/0x1a4

[2] Link: https://lore.kernel.org/all/[email protected]/

Reported-by Will Shiu <[email protected]>
Suggested-by Gao Xiang <[email protected]>
Signed-off-by Sandeep Dhavale <[email protected]>
Reviewed-by Gao Xiang <[email protected]>
Reviewed-by Alexandre Mergnat <[email protected]>
Link https://lore.kernel.org/r/[email protected] open here
Signed-off-by Gao Xiang <[email protected]>

[ Backport to 6.1.y: the source change is unchanged; only its location
in z_erofs_decompress_kickoff() differs. ]

Assisted-by LLM
Signed-off-by Artem Dinaburg <[email protected]>

---
Hi Greg, Sasha, and EROFS maintainers,

I am continuing backporting CVE fixes still missing from 6.1.y. This fix is inherited by v6.6 and every later mainline release, but 6.1.y still has the affected code. The target-specific adjustment is described in the bracketed note above.

Could you please queue it for 6.1.y?

Thanks,
Artem Dinaburg

CVE: CVE-2023-53231

Build: This patch was included in an x86_64 allmodconfig and CONFIG_WERROR=y build.
It produced vmlinux and modules with no new warnings or errors.

AI assistance: An LLM helped find, adapt, and validate this backport; I reviewed the patch and test output.

 fs/erofs/zdata.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/erofs/zdata.c +1 -1
index 66c323cbdd73e1..49b7c77488415a 100644
@@ -1271,7 +1271,7 @@ static void z_erofs_decompress_kickoff(struct z_erofs_decompressqueue *io,
 	if (atomic_add_return(bios, &io->pending_bios))
 		return;
 	/* Use workqueue and sync decompression for atomic contexts only */
-	if (in_atomic() || irqs_disabled()) {
+	if (!in_task() || irqs_disabled() || rcu_read_lock_any_held()) {
 		queue_work(z_erofs_workqueue, &io->u.work);
 		/* enable sync decompression for readahead */
 		if (sbi->opt.sync_decompress == EROFS_SYNC_DECOMPRESS_AUTO)
Signature · 1 line
2.39.5
maintainer Applied
#1 lore raw
Quoted 2 lines

[ Backport to 6.1.y: the source change is unchanged; only its location
in z_erofs_decompress_kickoff() differs. ]

Queued for 6.1, thanks.

Signature · 2 lines
Thanks,
Sasha
#2 lore raw
In reply to Artem Dinaburg

Hi Artem,

Quoted 82 lines · Artem Dinaburg

From: Sandeep Dhavale <[email protected]>

[ Upstream commit 12d0a24afd9ea58e581ea64d64e066f2027b28d9 ]

Current check for atomic context is not sufficient as z_erofs_decompressqueue_endio can be called under rcu lock from blk_mq_flush_plug_list(). See the stacktrace [1]

In such case we should hand off the decompression work for async processing rather than trying to do sync decompression in current context. Patch fixes the detection by checking for rcu_read_lock_any_held() and while at it use more appropriate !in_task() check than in_atomic().

Background: Historically erofs would always schedule a kworker for decompression which would incur the scheduling cost regardless of the context. But z_erofs_decompressqueue_endio() may not always be in atomic context and we could actually benefit from doing the decompression in z_erofs_decompressqueue_endio() if we are in thread context, for example when running with dm-verity. This optimization was later added in patch [2] which has shown improvement in performance benchmarks.

==============================================
[1] Problem stacktrace
[name:core&]BUG: sleeping function called from invalid context at kernel/locking/mutex.c:291 [name:core&]in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 1615, name: CpuMonitorServi [name:core&]preempt_count: 0, expected: 0
[name:core&]RCU nest depth: 1, expected: 0

CPU: 7 PID: 1615 Comm: CpuMonitorServi Tainted: G S      W  OE      6.1.25-android14-5-maybe-dirty-mainline #1
Hardware name: MT6897 (DT)
Call trace:
 dump_backtrace+0x108/0x15c
 show_stack+0x20/0x30
 dump_stack_lvl+0x6c/0x8c
 dump_stack+0x20/0x48
 __might_resched+0x1fc/0x308
 __might_sleep+0x50/0x88
 mutex_lock+0x2c/0x110
 z_erofs_decompress_queue+0x11c/0xc10
 z_erofs_decompress_kickoff+0x110/0x1a4
 z_erofs_decompressqueue_endio+0x154/0x180
 bio_endio+0x1b0/0x1d8
 __dm_io_complete+0x22c/0x280
 clone_endio+0xe4/0x280
 bio_endio+0x1b0/0x1d8
 blk_update_request+0x138/0x3a4
 blk_mq_plug_issue_direct+0xd4/0x19c
 blk_mq_flush_plug_list+0x2b0/0x354
 __blk_flush_plug+0x110/0x160
 blk_finish_plug+0x30/0x4c
 read_pages+0x2fc/0x370
 page_cache_ra_unbounded+0xa4/0x23c
 page_cache_ra_order+0x290/0x320
 do_sync_mmap_readahead+0x108/0x2c0
 filemap_fault+0x19c/0x52c
 __do_fault+0xc4/0x114
 handle_mm_fault+0x5b4/0x1168
 do_page_fault+0x338/0x4b4
 do_translation_fault+0x40/0x60
 do_mem_abort+0x60/0xc8
 el0_da+0x4c/0xe0
 el0t_64_sync_handler+0xd4/0xfc
 el0t_64_sync+0x1a0/0x1a4

[2] Link: https://lore.kernel.org/all/[email protected]/

Reported-by Will Shiu <[email protected]>
Suggested-by Gao Xiang <[email protected]>
Signed-off-by Sandeep Dhavale <[email protected]>
Reviewed-by Gao Xiang <[email protected]>
Reviewed-by Alexandre Mergnat <[email protected]>
Link https://lore.kernel.org/r/[email protected] open here
Signed-off-by Gao Xiang <[email protected]>

[ Backport to 6.1.y: the source change is unchanged; only its location
in z_erofs_decompress_kickoff() differs. ]

Assisted-by LLM
Signed-off-by Artem Dinaburg <[email protected]>

---
Hi Greg, Sasha, and EROFS maintainers,

Quoted 6 lines

I am continuing backporting CVE fixes still missing from 6.1.y. This fix is inherited by v6.6 and every later mainline release, but 6.1.y still has the affected code. The target-specific adjustment is described in the bracketed note above.

Could you please queue it for 6.1.y?

Sorry for late reply.

Thanks for your effort, this commit has a follow-up commit commit c99fab6e80b76422741d34aafc2f930a482afbdd

erofs: fix atomic context detection when !CONFIG_DEBUG_LOCK_ALLOC

could you please consider backport those to stable kernels as well?

Thanks,
Gao Xiang

Quoted 3 lines

Thanks,
Artem Dinaburg

maintainer
#3 lore raw
Quoted 6 lines

Thanks for your effort, this commit has a follow-up commit commit c99fab6e80b76422741d34aafc2f930a482afbdd

erofs: fix atomic context detection when !CONFIG_DEBUG_LOCK_ALLOC

could you please consider backport those to stable kernels as well?

c99fab6e80b7 ("erofs: fix atomic context detection when !CONFIG_DEBUG_LOCK_ALLOC") is queued for 6.1 together with this patch, and 6.6.y and 6.12.y already have it. I've also queued both commits for 5.15. 5.10 doesn't need either, since it always hands decompression off to the workqueue.

Signature · 2 lines
Thanks,
Sasha
#4 lore raw

Hi Sasha,

Quoted 11 lines · Sasha Levin
Quoted 6 lines

Thanks for your effort, this commit has a follow-up commit commit c99fab6e80b76422741d34aafc2f930a482afbdd

erofs: fix atomic context detection when !CONFIG_DEBUG_LOCK_ALLOC

could you please consider backport those to stable kernels as well?

c99fab6e80b7 ("erofs: fix atomic context detection when !CONFIG_DEBUG_LOCK_ALLOC") is queued for 6.1 together with this patch, and 6.6.y and 6.12.y already have it. I've also queued both commits for 5.15. 5.10 doesn't need either, since it always hands decompression off to the workqueue.

Thanks for the effort!

btw, just correct a bit 5.15.y also has commit 648f2de053a8 ("erofs: use workqueue decompression for atomic contexts only"), so

upstream commit 12d0a24afd9e ("erofs: Fix detection of atomic context")
upstream commit c99fab6e80b7 ("erofs: fix atomic context detection when !CONFIG_DEBUG_LOCK_ALLOC")

are also needed for 5.15.y.

Thanks,
Gao Xiang

Quoted 5 lines
Signature · 3 lines
-- 
Thanks,
Sasha
maintainer
#5 lore raw
Quoted 7 lines

btw, just correct a bit 5.15.y also has commit 648f2de053a8 ("erofs: use workqueue decompression for atomic contexts only"), so

upstream commit 12d0a24afd9e ("erofs: Fix detection of atomic context")
upstream commit c99fab6e80b7 ("erofs: fix atomic context detection when !CONFIG_DEBUG_LOCK_ALLOC")

are also needed for 5.15.y.

Right, both are already queued for 5.15 (and for 6.1).

Signature · 2 lines
Thanks,
Sasha