| Message ID | 20260918025416.604297-1-dhavale@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | ext4: fix race between inline dir conversion and folio read | expand |
> Commit 90f097b1403f ("ext4: refactor the inline directory conversion and > new directory codepaths") moved directory block initialization in > ext4_convert_inline_data_nolock() into ext4_init_dirblock(), unlocking > data_bh before setting BH_Uptodate. > > When block size is smaller than PAGE_SIZE, multiple buffer heads share a > single block device folio. If a concurrent read on that folio runs via > block_read_full_folio() while data_bh is unlocked and not uptodate, it > locks data_bh and submits a disk read for the uninitialized block. > > Because ext4_init_dirblock() initializes bh->b_data without holding the > buffer lock, the disk read races with and overwrites the initialized > directory entries and checksum tail, corrupting the directory block: > > EXT4-fs warning: ext4_dirblock_csum_verify:375: inode #...: > No space for directory leaf checksum. Please run e2fsck -D. > EXT4-fs error: __ext4_find_entry:1626: inode #...: > checksumming directory block 0 > > To fix this, mark data_bh uptodate before unlocking it in > ext4_convert_inline_data_nolock(), just like the regular file path > does. Additionally, hold lock_buffer(bh) in ext4_init_dirblock() while > populating the directory entries and checksum tail so any buffer > modifications are properly serialized. > > Fixes: 90f097b1403f ("ext4: refactor the inline directory conversion and new directory codepaths") > Cc: stable@vger.kernel.org > Assisted-by: Antigravity:gemini-3.8-flash > Signed-off-by: Sandeep Dhavale <dhavale@google.com> Sashiko has reviewed this patch and found no issues. It looks great!
On 9/18/2026 10:54 AM, Sandeep Dhavale wrote: > Commit 90f097b1403f ("ext4: refactor the inline directory conversion and > new directory codepaths") moved directory block initialization in > ext4_convert_inline_data_nolock() into ext4_init_dirblock(), unlocking > data_bh before setting BH_Uptodate. > > When block size is smaller than PAGE_SIZE, multiple buffer heads share a > single block device folio. If a concurrent read on that folio runs via > block_read_full_folio() while data_bh is unlocked and not uptodate, it > locks data_bh and submits a disk read for the uninitialized block. > > Because ext4_init_dirblock() initializes bh->b_data without holding the > buffer lock, the disk read races with and overwrites the initialized > directory entries and checksum tail, corrupting the directory block: > > EXT4-fs warning: ext4_dirblock_csum_verify:375: inode #...: > No space for directory leaf checksum. Please run e2fsck -D. > EXT4-fs error: __ext4_find_entry:1626: inode #...: > checksumming directory block 0 > > To fix this, mark data_bh uptodate before unlocking it in > ext4_convert_inline_data_nolock(), just like the regular file path > does. Additionally, hold lock_buffer(bh) in ext4_init_dirblock() while > populating the directory entries and checksum tail so any buffer > modifications are properly serialized. > > Fixes: 90f097b1403f ("ext4: refactor the inline directory conversion and new directory codepaths") > Cc: stable@vger.kernel.org > Assisted-by: Antigravity:gemini-3.8-flash > Signed-off-by: Sandeep Dhavale <dhavale@google.com> > --- > Note: > This issue was reported by an Android partner encountering kernel panics > in ext4_dirblock_csum_verify() ("No space for directory leaf checksum") > on 16KB page size kernels mounting a 4KB block size ext4 filesystem. > > We verified with a standalone reproducer that the race reproduces > deterministically on Iteration 0 (< 1s) on both ARM64 (16KB page size, > 4KB ext4 blocks) and upstream ext4-tree/dev on x86_64 (4KB page size, > 1KB ext4 blocks), and confirmed that this patch resolves the issue. I verified this patch on an MTK platform running Android 17 (kernel 6.18) with 16KB page size and 4KB ext4 block size: the script can reliably reproduce the issue. After applying the patch, the issue no longer reproduces. Reproduction log snippet: EXT4-fs warning (device loop54): ext4_dirblock_csum_verify:375: inode #15: comm repro_issue2: No space for directory leaf checksum. Please run e2fsck -D. EXT4-fs error (device loop54): __ext4_find_entry:1626: inode #15: comm repro_issue2: checksumming directory block 0 Tested-by: liuderong <liuderong@xiaomi.com> > fs/ext4/inline.c | 1 + > fs/ext4/namei.c | 2 ++ > 2 files changed, 3 insertions(+) > > diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c > index ceee69a66482..0aa14cee3310 100644 > --- a/fs/ext4/inline.c > +++ b/fs/ext4/inline.c > @@ -1167,6 +1167,7 @@ static int ext4_convert_inline_data_nolock(handle_t *handle, > error = ext4_handle_dirty_metadata(handle, > inode, data_bh); > } else { > + set_buffer_uptodate(data_bh); > unlock_buffer(data_bh); > inode->i_size = inode->i_sb->s_blocksize; > i_size_write(inode, inode->i_sb->s_blocksize); > diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c > index 3b9740c1c16d..6550102fb56b 100644 > --- a/fs/ext4/namei.c > +++ b/fs/ext4/namei.c > @@ -2933,6 +2933,7 @@ int ext4_init_dirblock(handle_t *handle, struct inode *inode, > if (ext4_has_feature_metadata_csum(inode->i_sb)) > csum_size = sizeof(struct ext4_dir_entry_tail); > > + lock_buffer(bh); > de->inode = cpu_to_le32(inode->i_ino); > de->name_len = 1; > de->rec_len = ext4_rec_len_to_disk(ext4_dir_rec_len(de->name_len, NULL), > @@ -2965,6 +2966,7 @@ int ext4_init_dirblock(handle_t *handle, struct inode *inode, > BUFFER_TRACE(dir_block, "call ext4_handle_dirty_metadata"); > set_buffer_uptodate(bh); > set_buffer_verified(bh); > + unlock_buffer(bh); > return ext4_handle_dirty_dirblock(handle, inode, bh); > } >
On Thu 17-09-26 19:54:15, Sandeep Dhavale wrote: > Commit 90f097b1403f ("ext4: refactor the inline directory conversion and > new directory codepaths") moved directory block initialization in > ext4_convert_inline_data_nolock() into ext4_init_dirblock(), unlocking > data_bh before setting BH_Uptodate. > > When block size is smaller than PAGE_SIZE, multiple buffer heads share a > single block device folio. If a concurrent read on that folio runs via > block_read_full_folio() while data_bh is unlocked and not uptodate, it > locks data_bh and submits a disk read for the uninitialized block. > > Because ext4_init_dirblock() initializes bh->b_data without holding the > buffer lock, the disk read races with and overwrites the initialized > directory entries and checksum tail, corrupting the directory block: > > EXT4-fs warning: ext4_dirblock_csum_verify:375: inode #...: > No space for directory leaf checksum. Please run e2fsck -D. > EXT4-fs error: __ext4_find_entry:1626: inode #...: > checksumming directory block 0 > > To fix this, mark data_bh uptodate before unlocking it in > ext4_convert_inline_data_nolock(), just like the regular file path > does. Additionally, hold lock_buffer(bh) in ext4_init_dirblock() while > populating the directory entries and checksum tail so any buffer > modifications are properly serialized. > > Fixes: 90f097b1403f ("ext4: refactor the inline directory conversion and new directory codepaths") > Cc: stable@vger.kernel.org > Assisted-by: Antigravity:gemini-3.8-flash > Signed-off-by: Sandeep Dhavale <dhavale@google.com> Good catch! At least the first paragraph below would be also useful in the commit changelog - the realworld impact is important information. But I guess Ted can modify the changelog on commit if he wants. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > Note: > This issue was reported by an Android partner encountering kernel panics > in ext4_dirblock_csum_verify() ("No space for directory leaf checksum") > on 16KB page size kernels mounting a 4KB block size ext4 filesystem. > > We verified with a standalone reproducer that the race reproduces > deterministically on Iteration 0 (< 1s) on both ARM64 (16KB page size, > 4KB ext4 blocks) and upstream ext4-tree/dev on x86_64 (4KB page size, > 1KB ext4 blocks), and confirmed that this patch resolves the issue. > > fs/ext4/inline.c | 1 + > fs/ext4/namei.c | 2 ++ > 2 files changed, 3 insertions(+) > > diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c > index ceee69a66482..0aa14cee3310 100644 > --- a/fs/ext4/inline.c > +++ b/fs/ext4/inline.c > @@ -1167,6 +1167,7 @@ static int ext4_convert_inline_data_nolock(handle_t *handle, > error = ext4_handle_dirty_metadata(handle, > inode, data_bh); > } else { > + set_buffer_uptodate(data_bh); > unlock_buffer(data_bh); > inode->i_size = inode->i_sb->s_blocksize; > i_size_write(inode, inode->i_sb->s_blocksize); > diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c > index 3b9740c1c16d..6550102fb56b 100644 > --- a/fs/ext4/namei.c > +++ b/fs/ext4/namei.c > @@ -2933,6 +2933,7 @@ int ext4_init_dirblock(handle_t *handle, struct inode *inode, > if (ext4_has_feature_metadata_csum(inode->i_sb)) > csum_size = sizeof(struct ext4_dir_entry_tail); > > + lock_buffer(bh); > de->inode = cpu_to_le32(inode->i_ino); > de->name_len = 1; > de->rec_len = ext4_rec_len_to_disk(ext4_dir_rec_len(de->name_len, NULL), > @@ -2965,6 +2966,7 @@ int ext4_init_dirblock(handle_t *handle, struct inode *inode, > BUFFER_TRACE(dir_block, "call ext4_handle_dirty_metadata"); > set_buffer_uptodate(bh); > set_buffer_verified(bh); > + unlock_buffer(bh); > return ext4_handle_dirty_dirblock(handle, inode, bh); > } > > -- > 2.55.0.1082.g2b9226bbc0-goog >
> > Good catch! At least the first paragraph below would be also useful in the > commit changelog - the realworld impact is important information. But I > guess Ted can modify the changelog on commit if he wants. Feel free to add: > > Reviewed-by: Jan Kara <jack@suse.cz> > > Honza > Hi Jan, Thanks for the review! Hi Ted, Is that something you can add while applying or shall I send v2 with just updated commit message? Thanks, Sandeep. > > --- > > Note: > > This issue was reported by an Android partner encountering kernel panics > > in ext4_dirblock_csum_verify() ("No space for directory leaf checksum") > > on 16KB page size kernels mounting a 4KB block size ext4 filesystem. > > > > We verified with a standalone reproducer that the race reproduces > > deterministically on Iteration 0 (< 1s) on both ARM64 (16KB page size, > > 4KB ext4 blocks) and upstream ext4-tree/dev on x86_64 (4KB page size, > > 1KB ext4 blocks), and confirmed that this patch resolves the issue. > > > > fs/ext4/inline.c | 1 + > > fs/ext4/namei.c | 2 ++ > > 2 files changed, 3 insertions(+)
diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c index ceee69a66482..0aa14cee3310 100644 --- a/fs/ext4/inline.c +++ b/fs/ext4/inline.c @@ -1167,6 +1167,7 @@ static int ext4_convert_inline_data_nolock(handle_t *handle, error = ext4_handle_dirty_metadata(handle, inode, data_bh); } else { + set_buffer_uptodate(data_bh); unlock_buffer(data_bh); inode->i_size = inode->i_sb->s_blocksize; i_size_write(inode, inode->i_sb->s_blocksize); diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c index 3b9740c1c16d..6550102fb56b 100644 --- a/fs/ext4/namei.c +++ b/fs/ext4/namei.c @@ -2933,6 +2933,7 @@ int ext4_init_dirblock(handle_t *handle, struct inode *inode, if (ext4_has_feature_metadata_csum(inode->i_sb)) csum_size = sizeof(struct ext4_dir_entry_tail); + lock_buffer(bh); de->inode = cpu_to_le32(inode->i_ino); de->name_len = 1; de->rec_len = ext4_rec_len_to_disk(ext4_dir_rec_len(de->name_len, NULL), @@ -2965,6 +2966,7 @@ int ext4_init_dirblock(handle_t *handle, struct inode *inode, BUFFER_TRACE(dir_block, "call ext4_handle_dirty_metadata"); set_buffer_uptodate(bh); set_buffer_verified(bh); + unlock_buffer(bh); return ext4_handle_dirty_dirblock(handle, inode, bh); }
Commit 90f097b1403f ("ext4: refactor the inline directory conversion and new directory codepaths") moved directory block initialization in ext4_convert_inline_data_nolock() into ext4_init_dirblock(), unlocking data_bh before setting BH_Uptodate. When block size is smaller than PAGE_SIZE, multiple buffer heads share a single block device folio. If a concurrent read on that folio runs via block_read_full_folio() while data_bh is unlocked and not uptodate, it locks data_bh and submits a disk read for the uninitialized block. Because ext4_init_dirblock() initializes bh->b_data without holding the buffer lock, the disk read races with and overwrites the initialized directory entries and checksum tail, corrupting the directory block: EXT4-fs warning: ext4_dirblock_csum_verify:375: inode #...: No space for directory leaf checksum. Please run e2fsck -D. EXT4-fs error: __ext4_find_entry:1626: inode #...: checksumming directory block 0 To fix this, mark data_bh uptodate before unlocking it in ext4_convert_inline_data_nolock(), just like the regular file path does. Additionally, hold lock_buffer(bh) in ext4_init_dirblock() while populating the directory entries and checksum tail so any buffer modifications are properly serialized. Fixes: 90f097b1403f ("ext4: refactor the inline directory conversion and new directory codepaths") Cc: stable@vger.kernel.org Assisted-by: Antigravity:gemini-3.8-flash Signed-off-by: Sandeep Dhavale <dhavale@google.com> --- Note: This issue was reported by an Android partner encountering kernel panics in ext4_dirblock_csum_verify() ("No space for directory leaf checksum") on 16KB page size kernels mounting a 4KB block size ext4 filesystem. We verified with a standalone reproducer that the race reproduces deterministically on Iteration 0 (< 1s) on both ARM64 (16KB page size, 4KB ext4 blocks) and upstream ext4-tree/dev on x86_64 (4KB page size, 1KB ext4 blocks), and confirmed that this patch resolves the issue. fs/ext4/inline.c | 1 + fs/ext4/namei.c | 2 ++ 2 files changed, 3 insertions(+)