Re: [PATCH v3 13/33] hfs: don't set SB_RDONLY after MDB write errors

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] hfs: early return in hfs_file_truncate() corrupts inode and
  catalog sizes
--

commit bfa80e19c6c9ea6b12943abb2839781be7835d98
Author: Christian Brauner <brauner@kernel.org>

hfs: don't set SB_RDONLY after MDB write errors

Instead of setting SB_RDONLY without holding s_umount when an MDB write
fails, this commit introduces HFS_FLG_EMERGENCY_RO and rejects filesystem
modifications with -EROFS once the flag is set.

> diff --git a/fs/hfs/extent.c b/fs/hfs/extent.c
> index 26357086517fc..dacf7ce1ffe4a 100644
> --- a/fs/hfs/extent.c
> +++ b/fs/hfs/extent.c
[ ... ]
> @@ -491,6 +494,8 @@ void hfs_file_truncate(struct inode *inode)
>  	hfs_dbg("ino %llu, phys_size %llu -> i_size %llu\n",
>  		inode->i_ino, (long long)HFS_I(inode)->phys_size,
>  		inode->i_size);
> +	if (unlikely(hfs_emergency_ro(sb)))
> +		return;
>  	if (inode->i_size > HFS_I(inode)->phys_size) {
>  		struct address_space *mapping = inode->i_mapping;
>  		void *fsdata = NULL;

[Severity: Medium]
Can returning at the top of hfs_file_truncate() leave inode->i_size out
of sync with HFS_I(inode)->phys_size and HFS_I(inode)->alloc_blocks?

CPU0                                  CPU1
hfs_inode_setattr()
  hfs_emergency_ro(sb) -> false
  inode_dio_wait(inode)
                                      hfs_mdb_commit()
                                        set_bit(HFS_FLG_EMERGENCY_RO, ...)
  truncate_setsize(inode, ia_size)
  hfs_file_truncate(inode)
    if (hfs_emergency_ro(sb))
      return;  <-- skips i_size rollback
  mark_inode_dirty(inode)

When an expanding truncate hits -EROFS in hfs_write_begin(), the
inode->i_size > HFS_I(inode)->phys_size branch normally resets
inode->i_size = HFS_I(inode)->phys_size.  Returning before that branch
leaves inode->i_size expanded without allocated blocks, and because
hfs_write_inode() still runs after an MDB error, writeback persists
LgLen > PyLen into the catalog B-tree record via hfs_inode_write_fork().

Similarly, if an extending write allocates blocks in hfs_get_block() and
then fails a later block allocation after HFS_FLG_EMERGENCY_RO is set,
hfs_write_failed() -> hfs_file_truncate() returns before the out: label
resets phys_size, fs_blocks, and the inode byte count back to
inode->i_size.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-work-fs-sb_rdonly-v3-0-0be92bb73709@kernel.org?part=13