[PATCH 3/3] jffs2: fix GC thread BUG_ON during reconfigure via fspick

[Tristan Madani <tristmd@gmail.com>]

From: Tristan Madani <tristan@talencesecurity.com>

jffs2_do_remount_fs() uses fc->sb_flags to decide whether to start
the garbage collection thread.  However, when called via fspick(2)
followed by fsconfig(FSCONFIG_CMD_RECONFIGURE), fc->sb_flags does
not reflect the current mount state -- it only contains flags being
explicitly changed (as indicated by fc->sb_flags_mask).

When fspick() is called with flags=0 on a read-only mount,
fc->sb_flags has SB_RDONLY clear (since SB_RDONLY is not in
sb_flags_mask).  This causes jffs2_start_garbage_collect_thread()
to be called even though the filesystem remains read-only.  On the
second reconfigure, BUG_ON(c->gc_task) fires because the thread
from the first call is still running.

Fix this by computing the effective read-only state using both
fc->sb_flags and fc->sb_flags_mask.  Also unconditionally call
jffs2_stop_garbage_collect_thread() before potentially restarting
it, which is safe when gc_task is NULL and prevents the BUG_ON.

Reported-by: syzbot+61a9d95630970eece39d@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=61a9d95630970eece39d
Tested-by: syzbot+61a9d95630970eece39d@syzkaller.appspotmail.com
Fixes: ec10a24f10c8f ("vfs: Convert jffs2 to use the new mount API")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
 fs/jffs2/fs.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/jffs2/fs.c b/fs/jffs2/fs.c
index 6ada8369a7622..33574312b7abe 100644
--- a/fs/jffs2/fs.c
+++ b/fs/jffs2/fs.c
@@ -396,28 +396,28 @@ 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);
+	bool new_ro;
 
 	if (c->flags & JFFS2_SB_FLAG_RO && !sb_rdonly(sb))
 		return -EROFS;
 
-	/* We stop if it was running, then restart if it needs to.
-	   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 */
+	new_ro = (fc->sb_flags_mask & SB_RDONLY) ?
+		 (fc->sb_flags & SB_RDONLY) : sb_rdonly(sb);
+
+	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 (!new_ro)
 		jffs2_start_garbage_collect_thread(c);
 
 	fc->sb_flags |= SB_NOATIME;
 	return 0;
 }
-
 /* jffs2_new_inode: allocate a new inode and inocache, add it to the hash,
    fill in the raw_inode while you're at it. */
 struct inode *jffs2_new_inode (struct inode *dir_i, umode_t mode, struct jffs2_raw_inode *ri)
-- 
2.47.3


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/