[PATCH v3] ext4: fix discard work use-after-free on failed mount

[PATCH v3] ext4: fix discard work use-after-free on failed mount

[Fan Wu]

ext4_put_super() shuts the journal down before it releases the mballoc
structures, but the failure unwind of __ext4_fill_super() runs the two
steps in the opposite order: failed_mount6 calls ext4_mb_release(),
which flushes sbi->s_discard_work, and the journal is destroyed only
later, just above failed_mount3a.

With -o discard, that journal destroy re-arms the work after the flush:
ext4_journal_destroy() calls ext4_force_commit(), and the commit
callback, registered once mballoc is initialized, queues s_discard_work
whenever the discard option is set, even with an empty freed-data list.
A running transaction can be live at that point: replaying the orphan
list is the easiest way to get one, and the quota paths on
failed_mount8/failed_mount9 can leave one too. The final force commit
is not necessarily a no-op.

Nothing drains s_discard_work after that point: failed_mount3 flushes
only s_sb_upd_work and the s_err_report timer, and ext4_fill_super()
then frees sbi with a plain kfree() through ext4_free_sbi(). If the
system_dfl_wq worker is delayed across the rest of the unwind,
ext4_discard_work() then accesses the freed sbi, first through
sbi->s_sb and then while taking sbi->s_md_lock.

This is the pattern fixed for the s_err_report timer in commit
0ce160c5bdb6 ("ext4: fix timer use-after-free on failed mount"):
async state armed after the unwind's last drain point.

Force the commit at failed_mount6, before ext4_mb_release(), so that
the discard work its commit callback queues is normally already
pending when the flush_work() in ext4_mb_release() runs.
disable_work_sync() there then makes this airtight: it also drains an
instance the flush missed, and no later commit, including the force
commit inside the journal destroy further down the unwind, can
requeue the work. The journal destroy stays at its existing position.

This issue was found by an in-house static analysis tool.

Fixes: 55cdd0af2bc5 ("ext4: get discard out of jbd2 commit kthread contex")
Cc: stable@vger.kernel.org # v6.10+
Reviewed-by: Jan Kara <jack@suse.cz>
Assisted-by: Codex:gpt-5.6
Co-developed-by: Song Li <songl@zju.edu.cn>
Signed-off-by: Song Li <songl@zju.edu.cn>
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
v3: address review nits from Jan Kara: simplify the failed_mount6
    comment and drop the redundant sbi->s_journal check in
    ext4_force_commit()'s callers. Reviewed-by was obtained on v2.
v2: keep the journal shutdown at its existing point instead of
    destroying the journal at failed_mount6, per review from Jan Kara:
    force the outstanding transaction there so the re-arm happens
    before the ext4_mb_release() flush, and disable_work_sync() the
    discard work after the flush.
    https://lore.kernel.org/linux-ext4/20260909054124.657782-1-fanwu01@zju.edu.cn/
v1: https://lore.kernel.org/linux-ext4/20260820052102.4616-1-fanwu01@zju.edu.cn/
---
 fs/ext4/mballoc.c | 2 ++
 fs/ext4/super.c   | 6 ++++++
 2 files changed, 8 insertions(+)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index ed1bd00e11cd..874b71931ab6 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -3897,6 +3897,8 @@ void ext4_mb_release(struct super_block *sb)
 	 * wait the discard work to drain all of ext4_free_data
 	 */
 	flush_work(&sbi->s_discard_work);
+	/* Prevent the later journal teardown from requeueing discard work. */
+	disable_work_sync(&sbi->s_discard_work);
 	WARN_ON_ONCE(!list_empty(&sbi->s_discard_list));
 
 	group_info = rcu_access_pointer(sbi->s_group_info);
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 4b6112e5d6c5..37dc6b5f20c6 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -5761,6 +5761,12 @@ failed_mount8: __maybe_unused
 failed_mount7:
 	ext4_unregister_li_request(sb);
 failed_mount6:
+	/*
+	 * We can have a running transaction from orphan replay or quota
+	 * setup. Commit it so that discard work after commit runs before
+	 * we shutdown mballoc.
+	 */
+	ext4_force_commit(sb);
 	ext4_mb_release(sb);
 	ext4_flex_groups_free(sbi);
 failed_mount5:

Re: [PATCH v3] ext4: fix discard work use-after-free on failed mount

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Use-after-free of mballoc s_group_info during mount failure unwinding via deferred EA inode eviction.
--

commit 08ee0a465324ade857a129840a9cb84d583a4d78
Author: Fan Wu <fanwu01@zju.edu.cn>

ext4: fix discard work use-after-free on failed mount

ext4_put_super() shuts the journal down before it releases the mballoc
structures, but the failure unwind of __ext4_fill_super() runs the two
steps in the opposite order: failed_mount6 calls ext4_mb_release(),
which flushes sbi->s_discard_work, and the journal is destroyed only
later, just above failed_mount3a.

> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index bca0dc87d0b7c..459f69bf1c3a3 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -5776,6 +5776,12 @@ failed_mount8: __maybe_unused
>  failed_mount7:
>  	ext4_unregister_li_request(sb);
>  failed_mount6:
> +	/*
> +	 * We can have a running transaction from orphan replay or quota
> +	 * setup. Commit it so that discard work after commit runs before
> +	 * we shutdown mballoc.
> +	 */
> +	ext4_force_commit(sb);
>  	ext4_mb_release(sb);

[Severity: High]
This is a pre-existing issue, but does this lead to a use-after-free
of mballoc's s_group_info during deferred EA inode eviction?

If journal replay queues a deferred EA inode put (e.g., dropping the last
reference to an EA inode), it is placed on sbi->s_ea_inode_work. If a
subsequent initialization step fails before failed_mount9, the error
unwinding path calls ext4_mb_release() here, which frees sbi->s_group_info.

Later in the unwinding process at failed_mount_wq:

fs/ext4/super.c:__ext4_fill_super() {
	...
failed_mount_wq:
	/* Drain deferred EA inode iputs before freeing structures */
	flush_delayed_work(&sbi->s_ea_inode_work);
	...
}

This executes the deferred iput, resulting in:
ext4_evict_inode() -> ext4_free_blocks() -> ext4_mb_free_blocks()

Which dereferences the dangling s_group_info pointer:

fs/ext4/balloc.c:ext4_get_group_info() {
	...
	grp_info = sbi_array_rcu_deref(EXT4_SB(sb), s_group_info, indexv);
	...
}

Can this sequence occur with a crafted or corrupted filesystem image that
triggers both an EA inode eviction during journal replay and a subsequent
initialization failure?

>  	ext4_flex_groups_free(sbi);
>  failed_mount5:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909113703.698544-1-fanwu01@zju.edu.cn?part=1