Hi Fedor,
On 2025/3/2 18:56, Fedor Pchelkin wrote:
On Fri, 28. Feb 19:51, Alexey Panov wrote:
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]
Urgh, it doesn't look so minor indeed. Backward struct folio -> struct page conversions can be tricky sometimes. Please see several comments below.
I manually backported it for Linux 6.6.y, see https://web.git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/...
Actually I had a very similiar backport for Linux 6.1.y, but I forgot to send it out due to other ongoing stuffs.
I think this backport patch is all good, but you could also mention it follows linux 6.6.y conflict changes instead of "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
I'm looking at the diff of upstream commit and the first thing it does is to remove zeroing out the folio/page private field here:
// upstream commit 9e2f9d34dd12 ("erofs: handle overlapped pclusters out of crafted images properly") @@ -1450,7 +1451,6 @@ static void z_erofs_fill_bio_vec(struct bio_vec *bvec, * file-backed folios will be used instead. */ if (folio->private == (void *)Z_EROFS_PREALLOCATED_PAGE) {
folio->private = 0; tocache = true; goto out_tocache; }while in 6.1.129 the corresponding fragment seems untouched with the backport patch. Is it intended?
Yes, because it was added in commit 2080ca1ed3e4 ("erofs: tidy up `struct z_erofs_bvec`") and dropped again.
But for Linux 6.6.y and 6.1.y, we don't need to backport 2080ca1ed3e4.
Thanks, Gao Xiang