[PATCH] jffs2: don't start the GC thread on a read-only superblock

[Chaohai Chen]

jffs2_do_remount_fs() derives the read-write state from fc->sb_flags,
but fc->sb_flags is only meaningful for the bits selected by
fc->sb_flags_mask.

fspick() builds its fs_context with sb_flags=0 and sb_flags_mask=0,
which means "leave every superblock flag alone"; JFFS2 misreads that
as "the filesystem is becoming read-write" and starts jffs2_gcd_mtdX
although the superblock stays read-only, and reconfigure_super()
then leaves sb->s_flags untouched.

The thread then outlives its superblock: jffs2_kill_sb() stops the
garbage collect thread only when !sb_rdonly(sb), so kfree(c) runs
while the thread is still alive and the next GC pass dereferences
freed memory.

Compute the resulting read-only state from fc->sb_flags only when
SB_RDONLY is part of fc->sb_flags_mask, falling back to the current
state otherwise, and stop any running thread unconditionally before
deciding whether to restart it.

Fixes: ec10a24f10c8 ("vfs: Convert jffs2 to use the new mount API")
Reported-by: syzbot+49c659b5564b2a44083c@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://syzkaller.appspot.com/bug?extid=49c659b5564b2a44083c
Signed-off-by: Chaohai Chen <wdhh6@xxxxxxxxxx>
---
 fs/jffs2/fs.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/fs/jffs2/fs.c b/fs/jffs2/fs.c
index 05cf860307c7..6d04dfa7b910 100644
--- a/fs/jffs2/fs.c
+++ b/fs/jffs2/fs.c
@@ -396,6 +396,15 @@ void jffs2_dirty_inode(struct inode *inode, int flags)
 int jffs2_do_remount_fs(struct super_block *sb, struct fs_context *fc)
 {
 	struct jffs2_sb_info *c = JFFS2_SB_INFO(sb);
+	/*
+	 * fc->sb_flags is only meaningful for the bits selected by
+	 * fc->sb_flags_mask.  fspick() passes a mask of 0, which leaves the
+	 * read-only state untouched - that must not be read as "becoming
+	 * read-write", or we would spawn a GC thread on a superblock that
+	 * stays read-only (and which jffs2_kill_sb() would then not stop).
+	 */
+	bool will_be_ro = (fc->sb_flags_mask & SB_RDONLY) ?
+		!!(fc->sb_flags & SB_RDONLY) : sb_rdonly(sb);
 
 	if (c->flags & JFFS2_SB_FLAG_RO && !sb_rdonly(sb))
 		return -EROFS;
@@ -404,14 +413,14 @@ int jffs2_do_remount_fs(struct super_block *sb, struct fs_context *fc)
 	   This also catches the case where it was stopped and this
 	   is just a remount to restart it.
 	   Flush the writebuffer, if necessary, else we loose it */
+	jffs2_stop_garbage_collect_thread(c);
 	if (!sb_rdonly(sb)) {
-		jffs2_stop_garbage_collect_thread(c);
 		mutex_lock(&c->alloc_sem);
 		jffs2_flush_wbuf_pad(c);
 		mutex_unlock(&c->alloc_sem);
 	}
 
-	if (!(fc->sb_flags & SB_RDONLY))
+	if (!will_be_ro)
 		jffs2_start_garbage_collect_thread(c);
 
 	fc->sb_flags |= SB_NOATIME;
-- 
2.43.7