Commit 9e2f9d34dd12 ("erofs: handle overlapped pclusters out of crafted images properly") fixes CVE-2024-47736 [1] but brings another problem [2]. So commit 1a2180f6859c ("erofs: fix PSI memstall accounting") should be backported too.
[1] https://nvd.nist.gov/vuln/detail/CVE-2024-47736 [2] https://lore.kernel.org/all/CAKPOu+8tvSowiJADW2RuKyofL_CSkm_SuyZA7ME5vMLWmL6...
Gao Xiang (2): erofs: handle overlapped pclusters out of crafted images properly erofs: fix PSI memstall accounting
fs/erofs/zdata.c | 66 +++++++++++++++++++++++++----------------------- 1 file changed, 34 insertions(+), 32 deletions(-)
From: Gao Xiang hsiangkao@linux.alibaba.com
commit 9e2f9d34dd12e6e5b244ec488bcebd0c2d566c50 upstream.
syzbot reported a task hang issue due to a deadlock case where it is waiting for the folio lock of a cached folio that will be used for cache I/Os.
After looking into the crafted fuzzed image, I found it's formed with several overlapped big pclusters as below:
Ext: logical offset | length : physical offset | length 0: 0.. 16384 | 16384 : 151552.. 167936 | 16384 1: 16384.. 32768 | 16384 : 155648.. 172032 | 16384 2: 32768.. 49152 | 16384 : 537223168.. 537239552 | 16384 ...
Here, extent 0/1 are physically overlapped although it's entirely _impossible_ for normal filesystem images generated by mkfs.
First, managed folios containing compressed data will be marked as up-to-date and then unlocked immediately (unlike in-place folios) when compressed I/Os are complete. If physical blocks are not submitted in the incremental order, there should be separate BIOs to avoid dependency issues. However, the current code mis-arranges z_erofs_fill_bio_vec() and BIO submission which causes unexpected BIO waits.
Second, managed folios will be connected to their own pclusters for efficient inter-queries. However, this is somewhat hard to implement easily if overlapped big pclusters exist. Again, these only appear in fuzzed images so let's simply fall back to temporary short-lived pages for correctness.
Additionally, it justifies that referenced managed folios cannot be truncated for now and reverts part of commit 2080ca1ed3e4 ("erofs: tidy up `struct z_erofs_bvec`") for simplicity although it shouldn't be any difference.
Reported-by: syzbot+4fc98ed414ae63d1ada2@syzkaller.appspotmail.com Reported-by: syzbot+de04e06b28cfecf2281c@syzkaller.appspotmail.com Reported-by: syzbot+c8c8238b394be4a1087d@syzkaller.appspotmail.com Tested-by: syzbot+4fc98ed414ae63d1ada2@syzkaller.appspotmail.com Closes: https://lore.kernel.org/r/0000000000002fda01061e334873@google.com Fixes: 8e6c8fa9f2e9 ("erofs: enable big pcluster feature") Signed-off-by: Gao Xiang hsiangkao@linux.alibaba.com Link: https://lore.kernel.org/r/20240910070847.3356592-1-hsiangkao@linux.alibaba.c... [Alexey: minor fix to resolve merge conflict] Signed-off-by: Alexey Panov apanov@astralinux.ru --- Backport fix for CVE-2024-47736
fs/erofs/zdata.c | 59 +++++++++++++++++++++++++----------------------- 1 file changed, 31 insertions(+), 28 deletions(-)
diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c index 94e9e0bf3bbd..ac01c0ede7f7 100644 --- a/fs/erofs/zdata.c +++ b/fs/erofs/zdata.c @@ -1346,14 +1346,13 @@ static struct page *pickup_page_for_submission(struct z_erofs_pcluster *pcl, goto out;
lock_page(page); - - /* only true if page reclaim goes wrong, should never happen */ - DBG_BUGON(justfound && PagePrivate(page)); - - /* the page is still in manage cache */ - if (page->mapping == mc) { + if (likely(page->mapping == mc)) { WRITE_ONCE(pcl->compressed_bvecs[nr].page, page);
+ /* + * The cached folio is still in managed cache but without + * a valid `->private` pcluster hint. Let's reconnect them. + */ if (!PagePrivate(page)) { /* * impossible to be !PagePrivate(page) for @@ -1367,22 +1366,24 @@ static struct page *pickup_page_for_submission(struct z_erofs_pcluster *pcl, SetPagePrivate(page); }
- /* no need to submit io if it is already up-to-date */ - if (PageUptodate(page)) { - unlock_page(page); - page = NULL; + if (likely(page->private == (unsigned long)pcl)) { + /* don't submit cache I/Os again if already uptodate */ + if (PageUptodate(page)) { + unlock_page(page); + page = NULL; + + } + goto out; } - goto out; + /* + * Already linked with another pcluster, which only appears in + * crafted images by fuzzers for now. But handle this anyway. + */ + tocache = false; /* use temporary short-lived pages */ + } else { + DBG_BUGON(1); /* referenced managed folios can't be truncated */ + tocache = true; } - - /* - * the managed page has been truncated, it's unsafe to - * reuse this one, let's allocate a new cache-managed page. - */ - DBG_BUGON(page->mapping); - DBG_BUGON(!justfound); - - tocache = true; unlock_page(page); put_page(page); out_allocpage: @@ -1536,16 +1537,11 @@ static void z_erofs_submit_queue(struct z_erofs_decompress_frontend *f, end = cur + pcl->pclusterpages;
do { - struct page *page; - - page = pickup_page_for_submission(pcl, i++, pagepool, - mc); - if (!page) - continue; + struct page *page = NULL;
if (bio && (cur != last_index + 1 || last_bdev != mdev.m_bdev)) { -submit_bio_retry: +drain_io: submit_bio(bio); if (memstall) { psi_memstall_leave(&pflags); @@ -1554,6 +1550,13 @@ static void z_erofs_submit_queue(struct z_erofs_decompress_frontend *f, bio = NULL; }
+ if (!page) { + page = pickup_page_for_submission(pcl, i++, + pagepool, mc); + if (!page) + continue; + } + if (unlikely(PageWorkingset(page)) && !memstall) { psi_memstall_enter(&pflags); memstall = 1; @@ -1574,7 +1577,7 @@ static void z_erofs_submit_queue(struct z_erofs_decompress_frontend *f, }
if (bio_add_page(bio, page, PAGE_SIZE, 0) < PAGE_SIZE) - goto submit_bio_retry; + goto drain_io;
last_index = cur; bypass = false;
[ Sasha's backport helper bot ]
Hi,
Summary of potential issues: ⚠️ Found follow-up fixes in mainline
The upstream commit SHA1 provided is correct: 9e2f9d34dd12e6e5b244ec488bcebd0c2d566c50
WARNING: Author mismatch between patch and upstream commit: Backport author: Alexey Panovapanov@astralinux.ru Commit author: Gao Xianghsiangkao@linux.alibaba.com
Status in newer kernel trees: 6.13.y | Present (exact SHA1) 6.12.y | Present (exact SHA1) 6.6.y | Present (different SHA1: 1bf7e414cac3)
Found fixes commits: 1a2180f6859c erofs: fix PSI memstall accounting
Note: The patch differs from the upstream commit: --- 1: 9e2f9d34dd12e < -: ------------- erofs: handle overlapped pclusters out of crafted images properly -: ------------- > 1: 8b40df2f9aa65 erofs: handle overlapped pclusters out of crafted images properly ---
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-6.1.y | Success | Success |
From: Gao Xiang hsiangkao@linux.alibaba.com
commit 1a2180f6859c73c674809f9f82e36c94084682ba upstream.
Max Kellermann recently reported psi_group_cpu.tasks[NR_MEMSTALL] is incorrect in the 6.11.9 kernel.
The root cause appears to be that, since the problematic commit, bio can be NULL, causing psi_memstall_leave() to be skipped in z_erofs_submit_queue().
Reported-by: Max Kellermann max.kellermann@ionos.com Closes: https://lore.kernel.org/r/CAKPOu+8tvSowiJADW2RuKyofL_CSkm_SuyZA7ME5vMLWmL6pq... Fixes: 9e2f9d34dd12 ("erofs: handle overlapped pclusters out of crafted images properly") Reviewed-by: Chao Yu chao@kernel.org Signed-off-by: Gao Xiang hsiangkao@linux.alibaba.com Link: https://lore.kernel.org/r/20241127085236.3538334-1-hsiangkao@linux.alibaba.c... Signed-off-by: Alexey Panov apanov@astralinux.ru --- fs/erofs/zdata.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c index ac01c0ede7f7..d175b5d0a2f5 100644 --- a/fs/erofs/zdata.c +++ b/fs/erofs/zdata.c @@ -1589,11 +1589,10 @@ static void z_erofs_submit_queue(struct z_erofs_decompress_frontend *f, move_to_bypass_jobqueue(pcl, qtail, owned_head); } while (owned_head != Z_EROFS_PCLUSTER_TAIL);
- if (bio) { + if (bio) submit_bio(bio); - if (memstall) - psi_memstall_leave(&pflags); - } + if (memstall) + psi_memstall_leave(&pflags);
/* * although background is preferred, no one is pending for submission.
[ Sasha's backport helper bot ]
Hi,
✅ All tests passed successfully. No issues detected. No action required from the submitter.
The upstream commit SHA1 provided is correct: 1a2180f6859c73c674809f9f82e36c94084682ba
WARNING: Author mismatch between patch and upstream commit: Backport author: Alexey Panovapanov@astralinux.ru Commit author: Gao Xianghsiangkao@linux.alibaba.com
Status in newer kernel trees: 6.13.y | Present (exact SHA1) 6.12.y | Present (different SHA1: 0653fa6ee045) 6.6.y | Present (different SHA1: 14f030a807dd)
Note: The patch differs from the upstream commit: --- 1: 1a2180f6859c7 < -: ------------- erofs: fix PSI memstall accounting -: ------------- > 1: 63f49db1d3fb4 erofs: fix PSI memstall accounting ---
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-6.1.y | Success | Success |
On 2025/3/1 00:51, Alexey Panov wrote:
Commit 9e2f9d34dd12 ("erofs: handle overlapped pclusters out of crafted images properly") fixes CVE-2024-47736 [1] but brings another problem [2]. So commit 1a2180f6859c ("erofs: fix PSI memstall accounting") should be backported too.
[1] https://nvd.nist.gov/vuln/detail/CVE-2024-47736 [2] https://lore.kernel.org/all/CAKPOu+8tvSowiJADW2RuKyofL_CSkm_SuyZA7ME5vMLWmL6...
Thanks for your effort! Patches look good to me.
Thanks, Gao Xiang
linux-stable-mirror@lists.linaro.org