| Message ID | 20260919204812.2814531-1-benquike@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | ext4: clear EXT4_STATE_MAY_INLINE_DATA in swap_inode_boot_loader() | expand |
> swap_inode_boot_loader() (EXT4_IOC_SWAP_BOOT) checks > !ext4_has_inline_data(inode), but neither checks > ext4_has_inline_data(inode_bl) nor clears EXT4_STATE_MAY_INLINE_DATA on > inode and inode_bl after swapping their inode data. > > When a newly created empty file (which has EXT4_STATE_MAY_INLINE_DATA > set in memory) is swapped with a non-empty EXT4_BOOT_LOADER_INO that > already has extent blocks, the target inode retains > EXT4_STATE_MAY_INLINE_DATA == 1 alongside extent blocks. A subsequent > ftruncate() + small pwrite() + fsync() dirties folio 0 while converting > the inode to inline data, hitting > BUG_ON(ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA)) in > ext4_writepages(). > > Reject inline data on inode_bl and clear EXT4_STATE_MAY_INLINE_DATA on > both inodes in swap_inode_boot_loader(). > > Fixes: 67a11611e1a5 ("ext4: fix check of inode in swap_inode_boot_loader") > Assisted-by: LLM > Signed-off-by: Hui Peng <benquike@gmail.com> Sashiko has reviewed this patch and found no issues. It looks great!
On Sat 19-09-26 20:48:12, Hui Peng wrote: > swap_inode_boot_loader() (EXT4_IOC_SWAP_BOOT) checks > !ext4_has_inline_data(inode), but neither checks > ext4_has_inline_data(inode_bl) nor clears EXT4_STATE_MAY_INLINE_DATA on > inode and inode_bl after swapping their inode data. > > When a newly created empty file (which has EXT4_STATE_MAY_INLINE_DATA > set in memory) is swapped with a non-empty EXT4_BOOT_LOADER_INO that > already has extent blocks, the target inode retains > EXT4_STATE_MAY_INLINE_DATA == 1 alongside extent blocks. A subsequent > ftruncate() + small pwrite() + fsync() dirties folio 0 while converting > the inode to inline data, hitting > BUG_ON(ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA)) in > ext4_writepages(). > > Reject inline data on inode_bl and clear EXT4_STATE_MAY_INLINE_DATA on > both inodes in swap_inode_boot_loader(). > > Fixes: 67a11611e1a5 ("ext4: fix check of inode in swap_inode_boot_loader") > Assisted-by: LLM > Signed-off-by: Hui Peng <benquike@gmail.com> > --- > diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c > --- a/fs/ext4/ioctl.c > +++ b/fs/ext4/ioctl.c > @@ -397,7 +397,7 @@ static long swap_inode_boot_loader(struct super_block *sb, > if (inode->i_nlink != 1 || !S_ISREG(inode->i_mode) || > IS_SWAPFILE(inode) || IS_ENCRYPTED(inode) || > (EXT4_I(inode)->i_flags & EXT4_JOURNAL_DATA_FL) || > - ext4_has_inline_data(inode)) { > + ext4_has_inline_data(inode) || ext4_has_inline_data(inode_bl)) { > err = -EINVAL; > goto journal_err_out; > } I guess instead we should add a check to check_igot_inode() that inode doesn't have inline data if EXT4_IGET_SPECIAL flag is set. > @@ -461,6 +461,8 @@ static long swap_inode_boot_loader(struct super_block *sb, > size_bl = (qsize_t)(inode_bl->i_blocks) * (1 << 9) + inode_bl->i_bytes; > diff = size - size_bl; > swap_inode_data(inode, inode_bl); > + ext4_clear_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA); > + ext4_clear_inode_state(inode_bl, EXT4_STATE_MAY_INLINE_DATA); Please don't touch inode_bl. That one should never have EXT4_STATE_MAY_INLINE_DATA set (and nobody can manipulate it directly anyway). For 'inode' I agree it will get blocks from inode_bl so better just clear EXT4_STATE_MAY_INLINE_DATA. Honza
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c --- a/fs/ext4/ioctl.c +++ b/fs/ext4/ioctl.c @@ -397,7 +397,7 @@ static long swap_inode_boot_loader(struct super_block *sb, if (inode->i_nlink != 1 || !S_ISREG(inode->i_mode) || IS_SWAPFILE(inode) || IS_ENCRYPTED(inode) || (EXT4_I(inode)->i_flags & EXT4_JOURNAL_DATA_FL) || - ext4_has_inline_data(inode)) { + ext4_has_inline_data(inode) || ext4_has_inline_data(inode_bl)) { err = -EINVAL; goto journal_err_out; } @@ -461,6 +461,8 @@ static long swap_inode_boot_loader(struct super_block *sb, size_bl = (qsize_t)(inode_bl->i_blocks) * (1 << 9) + inode_bl->i_bytes; diff = size - size_bl; swap_inode_data(inode, inode_bl); + ext4_clear_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA); + ext4_clear_inode_state(inode_bl, EXT4_STATE_MAY_INLINE_DATA); inode_set_ctime_current(inode); inode_set_ctime_current(inode_bl);
swap_inode_boot_loader() (EXT4_IOC_SWAP_BOOT) checks !ext4_has_inline_data(inode), but neither checks ext4_has_inline_data(inode_bl) nor clears EXT4_STATE_MAY_INLINE_DATA on inode and inode_bl after swapping their inode data. When a newly created empty file (which has EXT4_STATE_MAY_INLINE_DATA set in memory) is swapped with a non-empty EXT4_BOOT_LOADER_INO that already has extent blocks, the target inode retains EXT4_STATE_MAY_INLINE_DATA == 1 alongside extent blocks. A subsequent ftruncate() + small pwrite() + fsync() dirties folio 0 while converting the inode to inline data, hitting BUG_ON(ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA)) in ext4_writepages(). Reject inline data on inode_bl and clear EXT4_STATE_MAY_INLINE_DATA on both inodes in swap_inode_boot_loader(). Fixes: 67a11611e1a5 ("ext4: fix check of inode in swap_inode_boot_loader") Assisted-by: LLM Signed-off-by: Hui Peng <benquike@gmail.com> ---