Received-SPF: pass (google.com: domain of linux-kernel+bounces-72297-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <tencent_E860EA86EF0ECC0079FA6D3C2D496D30940A@qq.com>
From: Edward Adam Davis <eadavis@qq.com>
To: syzbot+c244f4a09ca85dd2ebc1@syzkaller.appspotmail.com
Cc: jfs-discussion@lists.sourceforge.net,
	linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	shaggy@kernel.org,
	syzkaller-bugs@googlegroups.com
Subject: [PATCH] jfs: fix uaf in jfs_syncpt
Date: Tue, 20 Feb 2024 11:55:18 +0800
In-Reply-To: <0000000000003d021006119cbf46@google.com>
References: <0000000000003d021006119cbf46@google.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

During the execution of the jfs lazy commit, the jfs file system was unmounted,
causing the sbi and jfs log objects to be released, triggering this issue.
The solution is to add mutex to synchronize jfs lazy commit and jfs unmount 
operations.

Reported-and-tested-by: syzbot+c244f4a09ca85dd2ebc1@syzkaller.appspotmail.com
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
 fs/jfs/jfs_incore.h | 1 +
 fs/jfs/jfs_txnmgr.c | 7 ++++++-
 fs/jfs/jfs_umount.c | 2 ++
 fs/jfs/super.c      | 1 +
 4 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/fs/jfs/jfs_incore.h b/fs/jfs/jfs_incore.h
index dd4264aa9bed..15955dd86bfd 100644
--- a/fs/jfs/jfs_incore.h
+++ b/fs/jfs/jfs_incore.h
@@ -197,6 +197,7 @@ struct jfs_sb_info {
 	kgid_t		gid;		/* gid to override on-disk gid */
 	uint		umask;		/* umask to override on-disk umask */
 	uint		minblks_trim;	/* minimum blocks, for online trim */
+	struct mutex log_mutex;
 };
 
 /* jfs_sb_info commit_state */
diff --git a/fs/jfs/jfs_txnmgr.c b/fs/jfs/jfs_txnmgr.c
index be17e3c43582..eb60862dc61b 100644
--- a/fs/jfs/jfs_txnmgr.c
+++ b/fs/jfs/jfs_txnmgr.c
@@ -2665,6 +2665,9 @@ static void txLazyCommit(struct tblock * tblk)
 
 	log = (struct jfs_log *) JFS_SBI(tblk->sb)->log;
 
+	if (!log)
+		return;
+
 	spin_lock_irq(&log->gclock);	// LOGGC_LOCK
 
 	tblk->flag |= tblkGC_COMMITTED;
@@ -2730,10 +2733,12 @@ int jfs_lazycommit(void *arg)
 				list_del(&tblk->cqueue);
 
 				LAZY_UNLOCK(flags);
+				mutex_lock(&sbi->log_mutex);
 				txLazyCommit(tblk);
+				sbi->commit_state &= ~IN_LAZYCOMMIT;
+				mutex_unlock(&sbi->log_mutex);
 				LAZY_LOCK(flags);
 
-				sbi->commit_state &= ~IN_LAZYCOMMIT;
 				/*
 				 * Don't continue in the for loop.  (We can't
 				 * anyway, it's unsafe!)  We want to go back to
diff --git a/fs/jfs/jfs_umount.c b/fs/jfs/jfs_umount.c
index 8ec43f53f686..04788cf3a471 100644
--- a/fs/jfs/jfs_umount.c
+++ b/fs/jfs/jfs_umount.c
@@ -51,6 +51,7 @@ int jfs_umount(struct super_block *sb)
 	 *
 	 * if mounted read-write and log based recovery was enabled
 	 */
+	mutex_lock(&sbi->log_mutex);
 	if ((log = sbi->log))
 		/*
 		 * Wait for outstanding transactions to be written to log:
@@ -113,6 +114,7 @@ int jfs_umount(struct super_block *sb)
 		 */
 		rc = lmLogClose(sb);
 	}
+	mutex_unlock(&sbi->log_mutex);
 	jfs_info("UnMount JFS Complete: rc = %d", rc);
 	return rc;
 }
diff --git a/fs/jfs/super.c b/fs/jfs/super.c
index 8d8e556bd610..cf291bdd094f 100644
--- a/fs/jfs/super.c
+++ b/fs/jfs/super.c
@@ -504,6 +504,7 @@ static int jfs_fill_super(struct super_block *sb, void *data, int silent)
 	sbi->uid = INVALID_UID;
 	sbi->gid = INVALID_GID;
 	sbi->umask = -1;
+	mutex_init(&sbi->log_mutex);
 
 	/* initialize the mount flag and determine the default error handler */
 	flag = JFS_ERR_REMOUNT_RO;
-- 
2.43.0