[PATCH RFC] jfs: fix null-ptr-deref and use-after-free during rw remount
"syzbot" <[email protected]>
| Newsgroups | dev.linux.lists.syzbot |
|---|---|
| Message-ID | <[email protected]> |
During a read-write remount of a JFS filesystem, jfs_mount_rw() destroys
the old in-memory inode and block maps (imap and bmap) before attempting to
read the new ones from disk. If dbMount() fails, the filesystem is left
with a NULL sbi->bmap. Any subsequent operation that triggers a lookup will
call diRead(), which dereferences sbi->bmap to access db_agl2size,
resulting in a NULL pointer dereference.
To fix this, jfs_mount_rw() is modified to allocate and mount the new maps
before freeing the old ones. If the new mount fails, the old maps are
restored.
Additionally, concurrent read operations like diRead() and jfs_statfs()
could access the old or new structures while they are being replaced,
leading to a use-after-free. To prevent this, the IREAD_LOCK in diRead() is
extended to protect the structures throughout their usage, and an
IREAD_LOCK is added to jfs_statfs() to protect its lockless accesses.
Furthermore, jfs_mount_rw() now holds the IWRITE_LOCK across the entire
remount process to ensure mutual exclusion with concurrent readers.
Fixes: a60dca73a1a8 ("jfs: makes diUnmount/diMount in jfs_mount_rw atomic")
Assisted-by: Gemini:gemini-3.1-pro-preview Gemini:gemini-3-flash-preview syzbot
Reported-by: [email protected]
Closes: https://syzkaller.appspot.com/bug?extid=8f731999dc47797f064f
Link: https://syzkaller.appspot.com/ai_job?id=4a947787-2a28-4919-858f-c3b11d2d0dbc
To: <[email protected]>
To: "Dave Kleikamp" <[email protected]>
Cc: "Christian Brauner" <[email protected]>
Cc: "Chuck Lever" <[email protected]>
Cc: "Jan Kara" <[email protected]>
Cc: "Jeff Layton" <[email protected]>
Cc: =?utf-8?q?Jo=C3=A3o_Paredes?= <[email protected]>
Cc: "Kees Cook" <[email protected]>
Cc: "Liao Yuanhong" <[email protected]>
Cc: <[email protected]>
Cc: "Al Viro" <[email protected]>
Cc: "Yun Zhou" <[email protected]>
---
diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
index b84ba4d7d..b0997e43f 100644
--- a/fs/jfs/jfs_imap.c
+++ b/fs/jfs/jfs_imap.c
@@ -314,8 +314,8 @@ int diRead(struct inode *ip)
IREAD_LOCK(ipimap, RDWRLOCK_IMAP);
imap = JFS_IP(ipimap)->i_imap;
rc = diIAGRead(imap, iagno, &mp);
- IREAD_UNLOCK(ipimap);
if (rc) {
+ IREAD_UNLOCK(ipimap);
jfs_err("diRead: diIAGRead returned %d", rc);
return (rc);
}
@@ -329,6 +329,7 @@ int diRead(struct inode *ip)
if ((lengthPXD(&iagp->inoext[extno]) != imap->im_nbperiext) ||
(addressPXD(&iagp->inoext[extno]) == 0)) {
release_metapage(mp);
+ IREAD_UNLOCK(ipimap);
return -ESTALE;
}
@@ -341,6 +342,7 @@ int diRead(struct inode *ip)
agstart = le64_to_cpu(iagp->agstart);
agno = BLKTOAG(agstart, JFS_SBI(ip->i_sb));
+ IREAD_UNLOCK(ipimap);
release_metapage(mp);
if (agno >= MAXAG || agno < 0)
return -EIO;
diff --git a/fs/jfs/jfs_mount.c b/fs/jfs/jfs_mount.c
index dac822f15..6229f50c0 100644
--- a/fs/jfs/jfs_mount.c
+++ b/fs/jfs/jfs_mount.c
@@ -229,26 +229,37 @@ int jfs_mount_rw(struct super_block *sb, int remount)
* them.
*/
if (remount) {
+ struct bmap *old_bmap;
+ struct inomap *old_imap;
+
if (chkSuper(sb) || (sbi->state != FM_CLEAN))
return -EINVAL;
+ old_bmap = sbi->bmap;
+ old_imap = JFS_IP(sbi->ipimap)->i_imap;
+
truncate_inode_pages(sbi->ipimap->i_mapping, 0);
truncate_inode_pages(sbi->ipbmap->i_mapping, 0);
IWRITE_LOCK(sbi->ipimap, RDWRLOCK_IMAP);
- diUnmount(sbi->ipimap, 1);
if ((rc = diMount(sbi->ipimap))) {
IWRITE_UNLOCK(sbi->ipimap);
jfs_err("jfs_mount_rw: diMount failed!");
return rc;
}
- IWRITE_UNLOCK(sbi->ipimap);
- dbUnmount(sbi->ipbmap, 1);
if ((rc = dbMount(sbi->ipbmap))) {
jfs_err("jfs_mount_rw: dbMount failed!");
+ /* restore old imap */
+ kfree(JFS_IP(sbi->ipimap)->i_imap);
+ JFS_IP(sbi->ipimap)->i_imap = old_imap;
+ IWRITE_UNLOCK(sbi->ipimap);
return rc;
}
+ IWRITE_UNLOCK(sbi->ipimap);
+
+ kfree(old_bmap);
+ kfree(old_imap);
}
/*
diff --git a/fs/jfs/super.c b/fs/jfs/super.c
index 61575f739..c65f68555 100644
--- a/fs/jfs/super.c
+++ b/fs/jfs/super.c
@@ -120,9 +120,13 @@ static int jfs_statfs(struct dentry *dentry, struct kstatfs *buf)
{
struct jfs_sb_info *sbi = JFS_SBI(dentry->d_sb);
s64 maxinodes;
- struct inomap *imap = JFS_IP(sbi->ipimap)->i_imap;
+ struct inomap *imap;
jfs_info("In jfs_statfs");
+
+ IREAD_LOCK(sbi->ipimap, RDWRLOCK_IMAP);
+ imap = JFS_IP(sbi->ipimap)->i_imap;
+
buf->f_type = JFS_SUPER_MAGIC;
buf->f_bsize = sbi->bsize;
buf->f_blocks = sbi->bmap->db_mapsize;
@@ -150,6 +154,7 @@ static int jfs_statfs(struct dentry *dentry, struct kstatfs *buf)
sizeof(sbi->uuid)/2);
buf->f_namelen = JFS_NAME_MAX;
+ IREAD_UNLOCK(sbi->ipimap);
return 0;
}
base-commit: e7ae89a0c97ce2b68b0983cd01eda67cf373517d
--
This is an AI-generated patch subject to moderation.
Reply with '#syz upstream' to Sign-off the patch as a human author
and send it to the upstream kernel mailing lists.
Reply with '#syz reject' to reject it ('#syz unreject' to undo).
See https://goo.gle/syzbot-ai-patches for information about AI-generated patches.
You can comment on the patch as usual, syzbot will try to address
the comments and send a new version of the patch if necessary.
syzbot engineers can be reached at [email protected].