Re: [PATCH] jffs2: fix use-after-free in jffs2_garbage_collect_thread

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] jffs2: uninitialized spinlock in jffs2_kill_sb() on rp_size mount error
--

commit 8866980c81747d68f6037e251702023d6140fd9d
Author: Yiqing Wu <wuyiqing@cmss.chinamobile.com>

jffs2: fix use-after-free in jffs2_garbage_collect_thread

This commit fixes a use-after-free of struct jffs2_sb_info by computing the
effective superblock flags in jffs2_do_remount_fs() before starting the
garbage collection thread, and by unconditionally calling
jffs2_stop_garbage_collect_thread() in jffs2_kill_sb().

[ ... ]

> 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);

[Severity: High]
Can calling jffs2_stop_garbage_collect_thread(c) unconditionally here lock an
uninitialized c->erase_completion_lock when a mount fails early in
jffs2_fill_super()?

During mount context initialization, jffs2_init_fs_context() in
fs/jffs2/super.c allocates struct jffs2_sb_info and assigns it to
fc->s_fs_info without initializing c->erase_completion_lock. When
get_tree_mtd()->mtd_get_sb()->sget_dev()->sget_fc() creates the superblock,
fc->s_fs_info is transferred to sb->s_fs_info before mtd_get_sb() calls
jffs2_fill_super(sb, fc).

In fs/jffs2/super.c:jffs2_fill_super():

	if (c->mount_opts.rp_size > c->mtd->size)
		return invalf(fc, "jffs2: Too large reserve pool specified, max is %llu KB",
			      c->mtd->size / 1024);

	/* Initialize JFFS2 superblock locks, the further initialization will
	 * be done later */
	mutex_init(&c->alloc_sem);
	mutex_init(&c->erase_free_sem);
	init_waitqueue_head(&c->erase_wait);
	init_waitqueue_head(&c->inocache_wq);
	spin_lock_init(&c->erase_completion_lock);

If a mount specifies an rp_size larger than c->mtd->size, jffs2_fill_super()
returns -EINVAL before spin_lock_init(&c->erase_completion_lock) is called.
mtd_get_sb() then calls deactivate_locked_super(sb)->jffs2_kill_sb(sb), where
c is non-NULL.

Previously, failed read-only mounts skipped jffs2_stop_garbage_collect_thread()
because sb_rdonly(sb) was true (though failed read-write mounts already hit
this path). With the !sb_rdonly(sb) check removed, any mount failing at the
rp_size check now calls jffs2_stop_garbage_collect_thread(c) in
fs/jffs2/background.c:

void jffs2_stop_garbage_collect_thread(struct jffs2_sb_info *c)
{
	int wait = 0;
	spin_lock(&c->erase_completion_lock);

Should c->erase_completion_lock be initialized earlier (for example, in
jffs2_init_fs_context() or before the rp_size check in jffs2_fill_super()) so
jffs2_kill_sb() can safely call jffs2_stop_garbage_collect_thread(c) on early
mount failure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103090018.157034-1-wuyiqing@cmss.chinamobile.com?part=1

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