* Re: [PATCH] jbd2: don't advance j_fc_off before the buffer is recorded
2026-08-19 6:09 ` [PATCH] jbd2: don't advance j_fc_off before the buffer is recorded Daejun Park
@ 2026-08-19 12:04 ` Jan Kara
2026-10-09 8:56 ` Theodore Ts'o
1 sibling, 0 replies; 3+ messages in thread
From: Jan Kara @ 2026-08-19 12:04 UTC (permalink / raw)
To: Daejun Park; +Cc: tytso, jack, jack, linux-ext4, linux-kernel, junzheyu1
On Wed 19-08-26 15:09:39, Daejun Park wrote:
> jbd2_fc_get_buf() increments journal->j_fc_off before jbd2_journal_bmap()
> and __getblk() have had a chance to fail. When either does, the slot
> j_fc_wbuf[fc_off] is never assigned, but j_fc_off already counts it.
>
> ext4 then fails the fast commit and falls back, and the fallback path
> reaches jbd2_fc_release_bufs() via ext4_fc_commit ->
> jbd2_fc_end_commit_fallback -> __jbd2_fc_end_commit -> ext4_fc_cleanup.
> That walks down from j_fc_off - 1 and put_bh()es every slot until it
> sees NULL, so it also touches the slot that was never written.
> j_fc_wbuf comes from a plain kmalloc() and is never zeroed, so a slot
> used for the first time holds uninitialised heap data.
>
> Advance j_fc_off only after the buffer head has been stored. That
> restores the invariant that j_fc_off covers exactly the filled slots,
> which is what jbd2_fc_release_bufs() relies on: the live buffers of the
> current fast commit sit above the NULLs the previous one released, so
> stopping at the first NULL is correct.
>
> Reproduced on a KASAN kernel with an ext4 fast_commit filesystem on a
> loop device, truncating the backing file under the live mount so that
> only the journal's fast-commit region falls past the end of the device.
> __getblk() then fails for the first fast-commit block while the rest of
> the journal is still addressable, so the journal is not aborted and the
> fast commit reaches the fallback path. j_fc_wbuf was poisoned to make
> the read of the never-written slot visible:
>
> BUG: KASAN: wild-memory-access in jbd2_fc_release_bufs+0x6b/0xd0
> Write of size 4 at addr 5a5a5a5a5a5a5ab2 by task sync/961
> jbd2_fc_release_bufs+0x6b/0xd0
> ext4_fc_cleanup+0x97/0x830
> __jbd2_fc_end_commit+0x37/0xc0
> ext4_fc_commit+0x524/0x560
> ext4_sync_file+0x3c4/0x4b0
> __x64_sys_fsync+0x20/0x30
>
> With this patch the same run leaves j_fc_off at 0 across repeated
> __getblk() failures and completes cleanly.
>
> Fixes: ff780b91efe9 ("jbd2: add fast commit machinery")
> Reported-by: Yu Junzhe <junzheyu1@gmail.com>
> Closes: https://lore.kernel.org/linux-ext4/7e4b109d-64c3-444f-a5dc-93c2ca4576cc@gmail.com/
> Signed-off-by: Daejun Park <daejun7.park@samsung.com>
Makes sense. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/jbd2/journal.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 00f5a98f3d4f..55b4ce010228 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -843,7 +843,6 @@ int jbd2_fc_get_buf(journal_t *journal, struct buffer_head **bh_out)
>
> fc_off = journal->j_fc_off;
> blocknr = journal->j_fc_first + fc_off;
> - journal->j_fc_off++;
> ret = jbd2_journal_bmap(journal, blocknr, &pblock);
> if (ret)
> return ret;
> @@ -853,6 +852,7 @@ int jbd2_fc_get_buf(journal_t *journal, struct buffer_head **bh_out)
> return -ENOMEM;
>
> journal->j_fc_wbuf[fc_off] = bh;
> + journal->j_fc_off++;
>
> *bh_out = bh;
>
>
> base-commit: 9091c97be34083587a75db174aab51551d8e8543
> --
> 2.43.0
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] jbd2: don't advance j_fc_off before the buffer is recorded
2026-08-19 6:09 ` [PATCH] jbd2: don't advance j_fc_off before the buffer is recorded Daejun Park
2026-08-19 12:04 ` Jan Kara
@ 2026-10-09 8:56 ` Theodore Ts'o
1 sibling, 0 replies; 3+ messages in thread
From: Theodore Ts'o @ 2026-10-09 8:56 UTC (permalink / raw)
To: jack, jack, Daejun Park
Cc: Theodore Ts'o, linux-ext4, linux-kernel, junzheyu1
On Wed, 19 Aug 2026 15:09:39 +0900, Daejun Park wrote:
> jbd2_fc_get_buf() increments journal->j_fc_off before jbd2_journal_bmap()
> and __getblk() have had a chance to fail. When either does, the slot
> j_fc_wbuf[fc_off] is never assigned, but j_fc_off already counts it.
>
> ext4 then fails the fast commit and falls back, and the fallback path
> reaches jbd2_fc_release_bufs() via ext4_fc_commit ->
> jbd2_fc_end_commit_fallback -> __jbd2_fc_end_commit -> ext4_fc_cleanup.
> That walks down from j_fc_off - 1 and put_bh()es every slot until it
> sees NULL, so it also touches the slot that was never written.
> j_fc_wbuf comes from a plain kmalloc() and is never zeroed, so a slot
> used for the first time holds uninitialised heap data.
>
> [...]
Applied, thanks!
[1/1] jbd2: don't advance j_fc_off before the buffer is recorded
commit: 9f359c031885bb50add7d23c7a5a0e9755b134c9
Best regards,
--
Theodore Ts'o <tytso@mit.edu>
^ permalink raw reply [flat|nested] 3+ messages in thread