Re: [PATCH 0/2] ext4: fix race conditions and clean up locking of inline data writes

[Jan Kara]

Hi!

On Wed 01-07-26 14:59:41, Aditya Prakash Srivastava wrote:
> While the cover letter mentions only a clean compile, I can categorically
> confirm that I have run fstests, LTP, and a layered filesystem I/O
> testing matrix before submitting. Sorry about the confusion in the write-up.

Cool, thanks for confirmation! I was suspecting this was just unlucky
formulation but wanted to make sure... I'm sorry for the noise.

								Honza

> On Wed, Jul 1, 2026 at 2:33 PM Jan Kara <jack@xxxxxxx> wrote:
> >
> > On Tue 30-06-26 15:28:10, Aditya Srivastava wrote:
> > > From: Aditya Prakash Srivastava <aditya.ansh182@xxxxxxxxx>
> > >
> > > This patch series addresses the remaining race conditions and locking
> > > issues involved with inline data writes, implementing the clean
> > > state-communication design suggested by Jan Kara.
> > >
> > > Previously, `ext4_write_end()`, `ext4_journalled_write_end()`, and
> > > `ext4_da_write_end()` checked the inode state and the inline data flag
> > > directly to decide whether to finish writing inline data or to fall
> > > back to block writes. This is highly susceptible to TOCTOU race
> > > conditions where concurrent memory-mapped page faults
> > > (`ext4_page_mkwrite()`) can convert the inline data to an extent
> > > between `write_begin` and `write_end`. Since block buffers were not
> > > allocated in the inline path during `write_begin`, such fallbacks
> > > resulted in kernel crashes and NULL pointer dereferences because
> > > `folio_buffers(folio)` was NULL.
> > >
> > > The series cleans up and resolves these issues in two distinct steps:
> > >
> > > 1) Patch 1 introduces state tracking via the standard `fsdata`
> > >    parameter. By marking whether a write was prepared as inline
> > >    (`EXT4_WRITE_DATA_INLINE`) directly in the private per-write
> > >    `fsdata` during `write_begin`, the corresponding `write_end`
> > >    handlers can reliably decide whether to call
> > >    `ext4_write_inline_data_end()` or complete a normal extent write.
> > >    This eliminates the race-prone checks on the live inode state and
> > >    gets rid of crude fallback/retry hacks.
> > >
> > > 2) Patch 2 replaces a potential kernel panic
> > >    (`BUG_ON(!ext4_has_inline_data(inode))`) inside
> > >    `ext4_write_inline_data_end()` with a graceful retry error path.
> > >    If a concurrent conversion clears the inline flag right after the
> > >    `write_end` checks pass but before the xattr semaphore is acquired,
> > >    we gracefully release all held resources and return 0 (VFS retry) to
> > >    let the VFS safely retry the write from scratch.
> > >
> > > The series compiles clean against the latest linux-next/ext4 tree.
> >
> > This caught my eye: Do you actually do real testing of your patches? Like
> > using fstests / kvm-xfstests? Every patch author for ext4 is supposed to do
> > that before submitting his changes...
> >
> >                                                                 Honza
> >
> > > Aditya Prakash Srivastava (2):
> > >   ext4: use fsdata to track inline data write state
> > >   ext4: replace BUG_ON with graceful retry in ext4_write_inline_data_end
> > >
> > >  fs/ext4/ext4.h   |  1 +
> > >  fs/ext4/inline.c | 14 +++++++++++++-
> > >  fs/ext4/inode.c  | 22 +++++++++++++---------
> > >  3 files changed, 27 insertions(+), 10 deletions(-)
> > >
> > > --
> > > 2.47.3
> > --
> > Jan Kara <jack@xxxxxxxx>
> > SUSE Labs, CR
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR