diff mbox series

jfs: fix uaf in jfs_syncpt

Message ID tencent_E860EA86EF0ECC0079FA6D3C2D496D30940A@qq.com
State New
Headers
Series jfs: fix uaf in jfs_syncpt |

Commit Message

Edward Adam Davis Feb. 20, 2024, 3:55 a.m. UTC 
  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(-)

Comments

Matthew Wilcox Feb. 20, 2024, 4:07 a.m. UTC | #1 
On Tue, Feb 20, 2024 at 11:55:18AM +0800, Edward Adam Davis wrote:
> 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.

Why is that the solution?  LAZY_LOCK with IN_LAZYCOMMIT is supposed to
cover this.  Please be more verbose in your commit messages.  Describe
what is going wrong and why; that will allow people to understand why
this is the correct solution to the problem.
diff mbox series

Patch

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;