From c214006856ff52a8ff17ed8da52d50601d54f9ce Mon Sep 17 00:00:00 2001 From: Arnaud Lecomte Date: Thu, 24 Apr 2025 00:13:51 +0200 Subject: [PATCH 1/6] jfs: upper bound check of tree index in dbAllocAG When computing the tree index in dbAllocAG, we never check if we are out of bounds realative to the size of the stree. This could happen in a scenario where the filesystem metadata are corrupted. Reported-by: syzbot+cffd18309153948f3c3e@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=cffd18309153948f3c3e Tested-by: syzbot+cffd18309153948f3c3e@syzkaller.appspotmail.com Signed-off-by: Arnaud Lecomte Signed-off-by: Dave Kleikamp --- fs/jfs/jfs_dmap.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c index 35e063c9f3a4..5080d59089bd 100644 --- a/fs/jfs/jfs_dmap.c +++ b/fs/jfs/jfs_dmap.c @@ -1389,6 +1389,12 @@ dbAllocAG(struct bmap * bmp, int agno, s64 nblocks, int l2nb, s64 * results) (1 << (L2LPERCTL - (bmp->db_agheight << 1))) / bmp->db_agwidth; ti = bmp->db_agstart + bmp->db_agwidth * (agno & (agperlev - 1)); + if (ti < 0 || ti >= le32_to_cpu(dcp->nleafs)) { + jfs_error(bmp->db_ipbmap->i_sb, "Corrupt dmapctl page\n"); + release_metapage(mp); + return -EIO; + } + /* dmap control page trees fan-out by 4 and a single allocation * group may be described by 1 or 2 subtrees within the ag level * dmap control page, depending upon the ag size. examine the ag's From 2d04df8116426b6c7b9f8b9b371250f666a2a2fb Mon Sep 17 00:00:00 2001 From: Edward Adam Davis Date: Wed, 4 Jun 2025 14:48:43 +0800 Subject: [PATCH 2/6] jfs: Regular file corruption check The reproducer builds a corrupted file on disk with a negative i_size value. Add a check when opening this file to avoid subsequent operation failures. Reported-by: syzbot+630f6d40b3ccabc8e96e@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=630f6d40b3ccabc8e96e Tested-by: syzbot+630f6d40b3ccabc8e96e@syzkaller.appspotmail.com Signed-off-by: Edward Adam Davis Signed-off-by: Dave Kleikamp --- fs/jfs/file.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/jfs/file.c b/fs/jfs/file.c index 01b6912e60f8..742cadd1f37e 100644 --- a/fs/jfs/file.c +++ b/fs/jfs/file.c @@ -44,6 +44,9 @@ static int jfs_open(struct inode *inode, struct file *file) { int rc; + if (S_ISREG(inode->i_mode) && inode->i_size < 0) + return -EIO; + if ((rc = dquot_file_open(inode, file))) return rc; From 1014354cd8d40e81e9a6e9037a10ebfc0b656d27 Mon Sep 17 00:00:00 2001 From: Suchit Karunakaran Date: Thu, 5 Jun 2025 23:26:34 +0530 Subject: [PATCH 3/6] jfs: jfs_xtree: replace XT_GETPAGE macro with xt_getpage() Replace legacy XT_GETPAGE macro with an inline function that returns a xtpage_t pointer and update all instances of XT_GETPAGE in jfs_xtree.c Signed-off-by: Suchit Karunakaran Simplified xt_getpage by removing size and rc arguments. Signed-off-by: Dave Kleikamp --- fs/jfs/jfs_xtree.c | 142 +++++++++++++++++++++++++-------------------- 1 file changed, 78 insertions(+), 64 deletions(-) diff --git a/fs/jfs/jfs_xtree.c b/fs/jfs/jfs_xtree.c index 5ee618d17e77..28c3cf960c6f 100644 --- a/fs/jfs/jfs_xtree.c +++ b/fs/jfs/jfs_xtree.c @@ -49,26 +49,6 @@ #define XT_PAGE(IP, MP) BT_PAGE(IP, MP, xtpage_t, i_xtroot) -/* get page buffer for specified block address */ -/* ToDo: Replace this ugly macro with a function */ -#define XT_GETPAGE(IP, BN, MP, SIZE, P, RC) \ -do { \ - BT_GETPAGE(IP, BN, MP, xtpage_t, SIZE, P, RC, i_xtroot); \ - if (!(RC)) { \ - if ((le16_to_cpu((P)->header.nextindex) < XTENTRYSTART) || \ - (le16_to_cpu((P)->header.nextindex) > \ - le16_to_cpu((P)->header.maxentry)) || \ - (le16_to_cpu((P)->header.maxentry) > \ - (((BN) == 0) ? XTROOTMAXSLOT : PSIZE >> L2XTSLOTSIZE))) { \ - jfs_error((IP)->i_sb, \ - "XT_GETPAGE: xtree page corrupt\n"); \ - BT_PUTPAGE(MP); \ - MP = NULL; \ - RC = -EIO; \ - } \ - } \ -} while (0) - /* for consistency */ #define XT_PUTPAGE(MP) BT_PUTPAGE(MP) @@ -114,6 +94,42 @@ static int xtSplitPage(tid_t tid, struct inode *ip, struct xtsplit * split, static int xtSplitRoot(tid_t tid, struct inode *ip, struct xtsplit * split, struct metapage ** rmpp); +/* + * xt_getpage() + * + * function: get the page buffer for a specified block address. + * + * parameters: + * ip - pointer to the inode + * bn - block number (s64) of the xtree page to be retrieved; + * mp - pointer to a metapage pointer where the page buffer is returned; + * + * returns: + * A pointer to the xtree page (xtpage_t) on success, -EIO on error. + */ + +static inline xtpage_t *xt_getpage(struct inode *ip, s64 bn, struct metapage **mp) +{ + xtpage_t *p; + int rc; + + BT_GETPAGE(ip, bn, *mp, xtpage_t, PSIZE, p, rc, i_xtroot); + + if (rc) + return ERR_PTR(rc); + if ((le16_to_cpu(p->header.nextindex) < XTENTRYSTART) || + (le16_to_cpu(p->header.nextindex) > + le16_to_cpu(p->header.maxentry)) || + (le16_to_cpu(p->header.maxentry) > + ((bn == 0) ? XTROOTMAXSLOT : PSIZE >> L2XTSLOTSIZE))) { + jfs_error(ip->i_sb, "xt_getpage: xtree page corrupt\n"); + BT_PUTPAGE(*mp); + *mp = NULL; + return ERR_PTR(-EIO); + } + return p; +} + /* * xtLookup() * @@ -216,7 +232,6 @@ static int xtSearch(struct inode *ip, s64 xoff, s64 *nextp, int *cmpp, struct btstack * btstack, int flag) { struct jfs_inode_info *jfs_ip = JFS_IP(ip); - int rc = 0; int cmp = 1; /* init for empty page */ s64 bn; /* block number */ struct metapage *mp; /* page buffer */ @@ -252,9 +267,9 @@ static int xtSearch(struct inode *ip, s64 xoff, s64 *nextp, */ for (bn = 0;;) { /* get/pin the page to search */ - XT_GETPAGE(ip, bn, mp, PSIZE, p, rc); - if (rc) - return rc; + p = xt_getpage(ip, bn, &mp); + if (IS_ERR(p)) + return PTR_ERR(p); /* try sequential access heuristics with the previous * access entry in target leaf page: @@ -807,10 +822,10 @@ xtSplitUp(tid_t tid, * insert router entry in parent for new right child page */ /* get/pin the parent page */ - XT_GETPAGE(ip, parent->bn, smp, PSIZE, sp, rc); - if (rc) { + sp = xt_getpage(ip, parent->bn, &smp); + if (IS_ERR(sp)) { XT_PUTPAGE(rcmp); - return rc; + return PTR_ERR(sp); } /* @@ -1062,10 +1077,10 @@ xtSplitPage(tid_t tid, struct inode *ip, * update previous pointer of old next/right page of */ if (nextbn != 0) { - XT_GETPAGE(ip, nextbn, mp, PSIZE, p, rc); - if (rc) { + p = xt_getpage(ip, nextbn, &mp); + if (IS_ERR(p)) { XT_PUTPAGE(rmp); - goto clean_up; + return PTR_ERR(p); } BT_MARK_DIRTY(mp, ip); @@ -1417,9 +1432,9 @@ int xtExtend(tid_t tid, /* transaction id */ return rc; /* get back old page */ - XT_GETPAGE(ip, bn, mp, PSIZE, p, rc); - if (rc) - return rc; + p = xt_getpage(ip, bn, &mp); + if (IS_ERR(p)) + return PTR_ERR(p); /* * if leaf root has been split, original root has been * copied to new child page, i.e., original entry now @@ -1433,9 +1448,9 @@ int xtExtend(tid_t tid, /* transaction id */ XT_PUTPAGE(mp); /* get new child page */ - XT_GETPAGE(ip, bn, mp, PSIZE, p, rc); - if (rc) - return rc; + p = xt_getpage(ip, bn, &mp); + if (IS_ERR(p)) + return PTR_ERR(p); BT_MARK_DIRTY(mp, ip); if (!test_cflag(COMMIT_Nolink, ip)) { @@ -1711,9 +1726,9 @@ int xtUpdate(tid_t tid, struct inode *ip, xad_t * nxad) return rc; /* get back old page */ - XT_GETPAGE(ip, bn, mp, PSIZE, p, rc); - if (rc) - return rc; + p = xt_getpage(ip, bn, &mp); + if (IS_ERR(p)) + return PTR_ERR(p); /* * if leaf root has been split, original root has been * copied to new child page, i.e., original entry now @@ -1727,9 +1742,9 @@ int xtUpdate(tid_t tid, struct inode *ip, xad_t * nxad) XT_PUTPAGE(mp); /* get new child page */ - XT_GETPAGE(ip, bn, mp, PSIZE, p, rc); - if (rc) - return rc; + p = xt_getpage(ip, bn, &mp); + if (IS_ERR(p)) + return PTR_ERR(p); BT_MARK_DIRTY(mp, ip); if (!test_cflag(COMMIT_Nolink, ip)) { @@ -1788,9 +1803,9 @@ int xtUpdate(tid_t tid, struct inode *ip, xad_t * nxad) XT_PUTPAGE(mp); /* get new right page */ - XT_GETPAGE(ip, bn, mp, PSIZE, p, rc); - if (rc) - return rc; + p = xt_getpage(ip, bn, &mp); + if (IS_ERR(p)) + return PTR_ERR(p); BT_MARK_DIRTY(mp, ip); if (!test_cflag(COMMIT_Nolink, ip)) { @@ -1864,9 +1879,9 @@ printf("xtUpdate.updateLeft.split p:0x%p\n", p); return rc; /* get back old page */ - XT_GETPAGE(ip, bn, mp, PSIZE, p, rc); - if (rc) - return rc; + p = xt_getpage(ip, bn, &mp); + if (IS_ERR(p)) + return PTR_ERR(p); /* * if leaf root has been split, original root has been @@ -1881,9 +1896,9 @@ printf("xtUpdate.updateLeft.split p:0x%p\n", p); XT_PUTPAGE(mp); /* get new child page */ - XT_GETPAGE(ip, bn, mp, PSIZE, p, rc); - if (rc) - return rc; + p = xt_getpage(ip, bn, &mp); + if (IS_ERR(p)) + return PTR_ERR(p); BT_MARK_DIRTY(mp, ip); if (!test_cflag(COMMIT_Nolink, ip)) { @@ -2187,7 +2202,6 @@ void xtInitRoot(tid_t tid, struct inode *ip) */ s64 xtTruncate(tid_t tid, struct inode *ip, s64 newsize, int flag) { - int rc = 0; s64 teof; struct metapage *mp; xtpage_t *p; @@ -2268,9 +2282,9 @@ s64 xtTruncate(tid_t tid, struct inode *ip, s64 newsize, int flag) * first access of each page: */ getPage: - XT_GETPAGE(ip, bn, mp, PSIZE, p, rc); - if (rc) - return rc; + p = xt_getpage(ip, bn, &mp); + if (IS_ERR(p)) + return PTR_ERR(p); /* process entries backward from last index */ index = le16_to_cpu(p->header.nextindex) - 1; @@ -2506,9 +2520,9 @@ s64 xtTruncate(tid_t tid, struct inode *ip, s64 newsize, int flag) /* get back the parent page */ bn = parent->bn; - XT_GETPAGE(ip, bn, mp, PSIZE, p, rc); - if (rc) - return rc; + p = xt_getpage(ip, bn, &mp); + if (IS_ERR(p)) + return PTR_ERR(p); index = parent->index; @@ -2791,9 +2805,9 @@ s64 xtTruncate_pmap(tid_t tid, struct inode *ip, s64 committed_size) * first access of each page: */ getPage: - XT_GETPAGE(ip, bn, mp, PSIZE, p, rc); - if (rc) - return rc; + p = xt_getpage(ip, bn, &mp); + if (IS_ERR(p)) + return PTR_ERR(p); /* process entries backward from last index */ index = le16_to_cpu(p->header.nextindex) - 1; @@ -2836,9 +2850,9 @@ s64 xtTruncate_pmap(tid_t tid, struct inode *ip, s64 committed_size) /* get back the parent page */ bn = parent->bn; - XT_GETPAGE(ip, bn, mp, PSIZE, p, rc); - if (rc) - return rc; + p = xt_getpage(ip, bn, &mp); + if (IS_ERR(p)) + return PTR_ERR(p); index = parent->index; From 2d91b3765cd05016335cd5df5e5c6a29708ec058 Mon Sep 17 00:00:00 2001 From: Lizhi Xu Date: Fri, 13 Jun 2025 11:05:34 +0800 Subject: [PATCH 4/6] jfs: truncate good inode pages when hard link is 0 The fileset value of the inode copy from the disk by the reproducer is AGGR_RESERVED_I. When executing evict, its hard link number is 0, so its inode pages are not truncated. This causes the bugon to be triggered when executing clear_inode() because nrpages is greater than 0. Reported-by: syzbot+6e516bb515d93230bc7b@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=6e516bb515d93230bc7b Signed-off-by: Lizhi Xu Signed-off-by: Dave Kleikamp --- fs/jfs/inode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/jfs/inode.c b/fs/jfs/inode.c index 60fc92dee24d..81e6b18e81e1 100644 --- a/fs/jfs/inode.c +++ b/fs/jfs/inode.c @@ -145,9 +145,9 @@ void jfs_evict_inode(struct inode *inode) if (!inode->i_nlink && !is_bad_inode(inode)) { dquot_initialize(inode); + truncate_inode_pages_final(&inode->i_data); if (JFS_IP(inode)->fileset == FILESYSTEM_I) { struct inode *ipimap = JFS_SBI(inode->i_sb)->ipimap; - truncate_inode_pages_final(&inode->i_data); if (test_cflag(COMMIT_Freewmap, inode)) jfs_free_zero_link(inode); From b89798e79cf79b03af8797391f1f844efe924819 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Fri, 11 Jul 2025 10:12:32 +0200 Subject: [PATCH 5/6] jfs: stop using write_cache_pages Stop using the obsolete write_cache_pages and use writeback_iter directly. Signed-off-by: Christoph Hellwig Signed-off-by: Dave Kleikamp --- fs/jfs/jfs_metapage.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/fs/jfs/jfs_metapage.c b/fs/jfs/jfs_metapage.c index 9029cd216912..b98cf3bb6c1f 100644 --- a/fs/jfs/jfs_metapage.c +++ b/fs/jfs/jfs_metapage.c @@ -421,7 +421,7 @@ static void metapage_write_end_io(struct bio *bio) } static int metapage_write_folio(struct folio *folio, - struct writeback_control *wbc, void *unused) + struct writeback_control *wbc) { struct bio *bio = NULL; int block_offset; /* block offset of mp within page */ @@ -550,10 +550,12 @@ static int metapage_writepages(struct address_space *mapping, struct writeback_control *wbc) { struct blk_plug plug; + struct folio *folio = NULL; int err; blk_start_plug(&plug); - err = write_cache_pages(mapping, wbc, metapage_write_folio, NULL); + while ((folio = writeback_iter(mapping, wbc, folio, &err))) + err = metapage_write_folio(folio, wbc); blk_finish_plug(&plug); return err; @@ -813,7 +815,7 @@ static int metapage_write_one(struct folio *folio) if (folio_clear_dirty_for_io(folio)) { folio_get(folio); - ret = metapage_write_folio(folio, &wbc, NULL); + ret = metapage_write_folio(folio, &wbc); if (ret == 0) folio_wait_writeback(folio); folio_put(folio); From 856db37592021e9155384094e331e2d4589f28b1 Mon Sep 17 00:00:00 2001 From: Zheng Yu Date: Tue, 29 Jul 2025 01:22:14 +0000 Subject: [PATCH 6/6] jfs: fix metapage reference count leak in dbAllocCtl In dbAllocCtl(), read_metapage() increases the reference count of the metapage. However, when dp->tree.budmin < 0, the function returns -EIO without calling release_metapage() to decrease the reference count, leading to a memory leak. Add release_metapage(mp) before the error return to properly manage the metapage reference count and prevent the leak. Fixes: a5f5e4698f8abbb25fe4959814093fb5bfa1aa9d ("jfs: fix shift-out-of-bounds in dbSplit") Signed-off-by: Zheng Yu Signed-off-by: Dave Kleikamp --- fs/jfs/jfs_dmap.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c index 5080d59089bd..cdfa699cd7c8 100644 --- a/fs/jfs/jfs_dmap.c +++ b/fs/jfs/jfs_dmap.c @@ -1815,8 +1815,10 @@ dbAllocCtl(struct bmap * bmp, s64 nblocks, int l2nb, s64 blkno, s64 * results) return -EIO; dp = (struct dmap *) mp->data; - if (dp->tree.budmin < 0) + if (dp->tree.budmin < 0) { + release_metapage(mp); return -EIO; + } /* try to allocate the blocks. */