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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] disable_work_sync() on s_discard_work leaks ext4_free_data entries due to a race with the JBD2 commit callback
--

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

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

This commit fixes a use-after-free of sbi->s_discard_work on failed mounts.
It forces a journal commit before ext4_mb_release() and uses
disable_work_sync() to prevent the journal teardown from requeueing the
discard work.

> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index bca0dc87d0b7c..672aec4c3616f 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -5776,6 +5776,15 @@ failed_mount8: __maybe_unused
>  failed_mount7:
>  	ext4_unregister_li_request(sb);
>  failed_mount6:
> +	/*
> +	 * Flush any running transaction: its commit callback may queue
> +	 * s_discard_work, which the flush_work() in ext4_mb_release()
> +	 * below drains; disable_work_sync() there also drains an
> +	 * instance queued after that, and keeps the journal destroy
> +	 * further down the unwind from requeueing the work.
> +	 */
> +	if (sbi->s_journal)
> +		ext4_force_commit(sb);
>  	ext4_mb_release(sb);
>  	ext4_flex_groups_free(sbi);
>  failed_mount5:

[Severity: Medium]
Can ext4_force_commit() return before the JBD2 commit callback fully
completes?

If JBD2 updates j_commit_sequence before executing the callback, it appears
ext4_force_commit() can return early. This would allow the mount thread to
proceed to ext4_mb_release() while the JBD2 thread is concurrently running
the commit callback.

> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 06171a11db125..3f99efe5e2d5c 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -3898,6 +3898,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));

[Severity: Medium]
Does this sequence leak ext4_free_data structures if it races with the
JBD2 commit callback?

If flush_work() returns immediately (because the work isn't queued yet),
and disable_work_sync() is called while the JBD2 thread is executing
ext4_process_freed_data():

fs/ext4/mballoc.c:ext4_process_freed_data() {
	...
		list_splice_tail(&freed_data_list, &sbi->s_discard_list);
		spin_unlock(&sbi->s_md_lock);
		if (wake)
			queue_work(system_dfl_wq, &sbi->s_discard_work);
	...
}

Since the work is now disabled, queue_work() fails. Are the ext4_free_data
items spliced onto sbi->s_discard_list permanently stranded, leaking memory
and triggering the WARN_ON_ONCE() immediately following disable_work_sync()?

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