erofs: handle overlapped pclusters out of crafted images properly
BugLink: https://bugs.launchpad.net/bugs/2106770
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: 8e6c8fa9f2 ("erofs: enable big pcluster feature")
Link: https://lore.kernel.org/r/20240910070847.3356592-1-hsiangkao@linux.alibaba.com
Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
[diewald: combine changes of upstream commit 9e2f9d34dd12 and 6.6.y
stable backport 1bf7e414cac3 in 6.6.72]
CVE-2024-47736
Signed-off-by: Manuel Diewald <manuel.diewald@canonical.com>
Signed-off-by: Mehmet Basaran <mehmet.basaran@canonical.com>
This commit is contained in:
committed by
Mehmet Basaran
parent
6bacc0789f
commit
91fc6c3c09
+28
-29
@@ -1472,11 +1472,7 @@ repeat:
|
||||
}
|
||||
|
||||
lock_page(page);
|
||||
/* only true if page reclaim goes wrong, should never happen */
|
||||
DBG_BUGON(justfound && PagePrivate(page));
|
||||
|
||||
/* the cached page is still in managed cache */
|
||||
if (page->mapping == mc) {
|
||||
if (likely(page->mapping == mc)) {
|
||||
/*
|
||||
* The cached page is still available but without a valid
|
||||
* `->private` pcluster hint. Let's reconnect them.
|
||||
@@ -1487,23 +1483,23 @@ repeat:
|
||||
attach_page_private(page, pcl);
|
||||
put_page(page);
|
||||
}
|
||||
|
||||
/* no need to submit if it is already up-to-date */
|
||||
if (PageUptodate(page)) {
|
||||
unlock_page(page);
|
||||
bvec->bv_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);
|
||||
bvec->bv_page = NULL;
|
||||
}
|
||||
return;
|
||||
}
|
||||
return;
|
||||
/*
|
||||
* 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;
|
||||
}
|
||||
|
||||
/*
|
||||
* It has been truncated, so it's unsafe to reuse this one. Let's
|
||||
* allocate a new page for compressed data.
|
||||
*/
|
||||
DBG_BUGON(page->mapping);
|
||||
DBG_BUGON(!justfound);
|
||||
|
||||
tocache = true;
|
||||
unlock_page(page);
|
||||
put_page(page);
|
||||
out_allocpage:
|
||||
@@ -1659,13 +1655,10 @@ static void z_erofs_submit_queue(struct z_erofs_decompress_frontend *f,
|
||||
cur = mdev.m_pa;
|
||||
end = cur + pcl->pclustersize;
|
||||
do {
|
||||
z_erofs_fill_bio_vec(&bvec, f, pcl, i++, mc);
|
||||
if (!bvec.bv_page)
|
||||
continue;
|
||||
|
||||
bvec.bv_page = NULL;
|
||||
if (bio && (cur != last_pa ||
|
||||
last_bdev != mdev.m_bdev)) {
|
||||
submit_bio_retry:
|
||||
drain_io:
|
||||
submit_bio(bio);
|
||||
if (memstall) {
|
||||
psi_memstall_leave(&pflags);
|
||||
@@ -1674,6 +1667,15 @@ submit_bio_retry:
|
||||
bio = NULL;
|
||||
}
|
||||
|
||||
if (!bvec.bv_page) {
|
||||
z_erofs_fill_bio_vec(&bvec, f, pcl, i++, mc);
|
||||
if (!bvec.bv_page)
|
||||
continue;
|
||||
if (cur + bvec.bv_len > end)
|
||||
bvec.bv_len = end - cur;
|
||||
DBG_BUGON(bvec.bv_len < sb->s_blocksize);
|
||||
}
|
||||
|
||||
if (unlikely(PageWorkingset(bvec.bv_page)) &&
|
||||
!memstall) {
|
||||
psi_memstall_enter(&pflags);
|
||||
@@ -1692,12 +1694,9 @@ submit_bio_retry:
|
||||
last_bdev = mdev.m_bdev;
|
||||
}
|
||||
|
||||
if (cur + bvec.bv_len > end)
|
||||
bvec.bv_len = end - cur;
|
||||
DBG_BUGON(bvec.bv_len < sb->s_blocksize);
|
||||
if (!bio_add_page(bio, bvec.bv_page, bvec.bv_len,
|
||||
bvec.bv_offset))
|
||||
goto submit_bio_retry;
|
||||
goto drain_io;
|
||||
|
||||
last_pa = cur + bvec.bv_len;
|
||||
bypass = false;
|
||||
|
||||
Reference in New Issue
Block a user