[PATCH 1/3] jffs2: always stop garbage collection thread on unmount

[PATCH 1/3] jffs2: always stop garbage collection thread on unmount

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

jffs2_kill_sb() skips stopping the GC thread when the filesystem
is mounted read-only.  However, a filesystem can be remounted
read-only while the GC thread is still running.  In that case,
jffs2_stop_garbage_collect_thread() is never called, and the GC
thread continues to run after kfree(c), accessing freed memory.

The GC thread accesses c->gc_task, c->gc_mtd, and the full
jffs2_sb_info structure during jffs2_garbage_collect_pass().
After kfree(c), any of these accesses is a use-after-free.

Remove the sb_rdonly() check so the GC thread is always stopped
before freeing the superblock info.  jffs2_stop_garbage_collect_thread()
already handles the case where gc_task is NULL (no thread running),
so this is safe for the common case of a clean read-only mount.

Reported-by: syzbot+e84662c5f30b8c401437@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=e84662c5f30b8c401437
Tested-by: syzbot+e84662c5f30b8c401437@syzkaller.appspotmail.com
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
 fs/jffs2/super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
index 81396a092ba88..c846b435a38b6 100644
--- a/fs/jffs2/super.c
+++ b/fs/jffs2/super.c
@@ -345,7 +345,7 @@ static void jffs2_put_super (struct super_block *sb)
 static void jffs2_kill_sb(struct super_block *sb)
 {
 	struct jffs2_sb_info *c = JFFS2_SB_INFO(sb);
-	if (c && !sb_rdonly(sb))
+	if (c)
 		jffs2_stop_garbage_collect_thread(c);
 	kill_mtd_super(sb);
 	kfree(c);
-- 
2.47.3

[PATCH 2/3] jffs2: clean up xattr refs in jffs2_del_ino_cache instead of BUG_ON

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

jffs2_del_ino_cache() triggers BUG_ON(old->xref) when an inode cache
entry still has xattr references.  This can happen during unmount:
generic_shutdown_super() calls evict_inodes() before put_super(), but
jffs2_evict_inode -> jffs2_do_clear_inode -> jffs2_xattr_delete_inode
only clears xrefs when pino_nlink == 0.  For inodes with nlink > 0
at unmount time, xrefs survive past eviction, and the subsequent
jffs2_del_ino_cache() hits the BUG_ON.

Replace the BUG_ON with a call to jffs2_xattr_free_inode(), which
walks the xref list and frees each entry without writing delete markers
to flash.  This is appropriate during unmount since the flash state
will be reconstructed by the next mount scan anyway.

jffs2_xattr_free_inode() already exists for this purpose and is used
by jffs2_clear_xattr_subsystem() in the put_super path, but that
runs too late -- after jffs2_del_ino_cache has already been called
from evict_inode.

Reported-by: syzbot+44664704c1494ad5f7a0@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=44664704c1494ad5f7a0
Fixes: aa98d7cf59b5 ("[JFFS2][XATTR] XATTR support on JFFS2 (version 5)")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
 fs/jffs2/nodelist.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/jffs2/nodelist.c b/fs/jffs2/nodelist.c
index b86c78d178c60..9af269b78b241 100644
--- a/fs/jffs2/nodelist.c
+++ b/fs/jffs2/nodelist.c
@@ -459,7 +459,8 @@ void jffs2_del_ino_cache(struct jffs2_sb_info *c, struct jffs2_inode_cache *old)
 	struct jffs2_inode_cache **prev;
 
 #ifdef CONFIG_JFFS2_FS_XATTR
-	BUG_ON(old->xref);
+	if (old->xref)
+		jffs2_xattr_free_inode(c, old);
 #endif
 	dbg_inocache("del %p (ino #%u)\n", old, old->ino);
 	spin_lock(&c->inocache_lock);
-- 
2.47.3

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

[Tristan Madani]

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