Re: [PATCH] nilfs2: zero-fill post-EOF data on mmap write and size extension

[Viacheslav Dubeyko]

On Sat, 2026-11-07 at 01:45 +0900, Ryusuke Konishi wrote:
> When a file is mapped via mmap, writes beyond the EOF within the same
> page are permitted in memory.  However, POSIX requires that these
> modified portions beyond EOF must never be written to disk and must
> be
> zeroed if the file size is subsequently extended.
> 
> In nilfs_setattr(), size changes trigger truncate_setsize(), which
> calls
> pagecache_isize_extended() upon size extension.  However, this VFS
> helper
> does nothing when the page size is equal to the block size, when the
> extension stays within the same block, or when no block exists beyond
> the
> old EOF within the page, skipping the zeroing of the post-EOF region.
> The filesystem is therefore responsible for zeroing the post-EOF
> range in
> such cases.  However, the current implementation of nilfs_truncate()
> operates based on the updated i_size, leaving the post-EOF region of
> the
> mapped page unzeroed.
> 
> Fix this issue by handling size extension and contraction separately
> in
> nilfs_setattr().  When expanding a file, call block_truncate_page()
> with
> the old EOF position to zero the boundary block data beyond the
> original
> EOF, unless the old size is aligned to a block boundary, without
> truncating the bmap.  When shrinking a file, invoke nilfs_truncate()
> as
> before to release blocks beyond the new EOF and zero the boundary
> block.
> Propagate the error code from block_truncate_page() or
> nilfs_truncate()
> to nilfs_setattr() for proper error handling.
> 
> Additionally, move operations specific to explicit size modifications
> (permission and file type checks, timestamp updates, and sync flag
> handling) from nilfs_truncate() to nilfs_setattr().  This avoids
> executing them in the error recovery path of nilfs_write_failed() and
> ensures invalid size changes are rejected prior to calling
> truncate_setsize().
> 
> Reported-by: Vyacheslav Kovalevsky <slava.kovalevskiy.2014@xxxxxxxxx>
> Closes:
> https://lore.kernel.org/all/47d7da4a-5029-4c56-a5ac-3e4c526cc569@xxxxxxxxx
> Fixes: 05fe58fdc10d ("nilfs2: inode operations")
> Signed-off-by: Ryusuke Konishi <konishi.ryusuke@xxxxxxxxx>
> ---
> Hi Viacheslav,
> 
> Please apply this fix for the next cycle.
> 
> This addresses a POSIX compliance issue with mmap where extending a
> file leaves dirty data written beyond the original EOF unzeroed in
> the
> boundary block.
> 
> Thanks,
> Ryusuke Konishi
> 
>  fs/nilfs2/inode.c | 78 ++++++++++++++++++++++++++++++++++++++-------
> --
>  fs/nilfs2/nilfs.h |  1 -
>  2 files changed, 63 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
> index a99a69e9f91b..6ddef3ab0c19 100644
> --- a/fs/nilfs2/inode.c
> +++ b/fs/nilfs2/inode.c
> @@ -39,6 +39,7 @@ struct nilfs_iget_args {
>  };
>  
>  static int nilfs_iget_test(struct inode *inode, void *opaque);
> +static int nilfs_truncate(struct inode *inode);
>  
>  void nilfs_inode_add_blocks(struct inode *inode, int n)
>  {
> @@ -771,38 +772,35 @@ static void nilfs_truncate_bmap(struct
> nilfs_inode_info *ii,
>  		   ret, ii->vfs_inode.i_ino);
>  }
>  
> -void nilfs_truncate(struct inode *inode)
> +static int nilfs_truncate(struct inode *inode)
>  {
>  	unsigned long blkoff;
>  	unsigned int blocksize;
>  	struct nilfs_transaction_info ti;
>  	struct super_block *sb = inode->i_sb;
>  	struct nilfs_inode_info *ii = NILFS_I(inode);
> +	int err;
>  
>  	if (!test_bit(NILFS_I_BMAP, &ii->i_state))
> -		return;
> -	if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
> -		return;
> +		return -EINVAL;
>  
>  	blocksize = sb->s_blocksize;
>  	blkoff = (inode->i_size + blocksize - 1) >> sb-
> >s_blocksize_bits;
>  	nilfs_transaction_begin(sb, &ti, 0); /* never fails */
>  
> -	block_truncate_page(inode->i_mapping, inode->i_size,
> nilfs_get_block);
> +	err = block_truncate_page(inode->i_mapping, inode->i_size,
> +				nilfs_get_block);
> +	if (unlikely(err)) {
> +		nilfs_transaction_abort(sb);
> +		return err;
> +	}
>  
>  	nilfs_truncate_bmap(ii, blkoff);
>  
> -	inode_set_mtime_to_ts(inode,
> inode_set_ctime_current(inode));
> -	if (IS_SYNC(inode))
> -		nilfs_set_transaction_flag(NILFS_TI_SYNC);
> -
>  	nilfs_mark_inode_dirty(inode);
>  	nilfs_set_file_dirty(inode, 0);
>  	nilfs_transaction_commit(sb);
> -	/*
> -	 * May construct a logical segment and may fail in sync
> mode.
> -	 * But truncate has no return value.
> -	 */
> +	return 0;
>  }
>  
>  static void nilfs_clear_inode(struct inode *inode)
> @@ -884,6 +882,7 @@ int nilfs_setattr(const struct mnt_idmap *idmap,
> struct dentry *dentry,
>  	struct nilfs_transaction_info ti;
>  	struct inode *inode = d_inode(dentry);
>  	struct super_block *sb = inode->i_sb;
> +	loff_t oldsize;
>  	int err;
>  
>  	err = setattr_prepare(&nop_mnt_idmap, dentry, iattr);
> @@ -895,10 +894,59 @@ int nilfs_setattr(const struct mnt_idmap
> *idmap, struct dentry *dentry,
>  		return err;
>  
>  	if ((iattr->ia_valid & ATTR_SIZE) &&
> -	    iattr->ia_size != i_size_read(inode)) {
> +		iattr->ia_size != (oldsize = i_size_read(inode))) {
> +
> +		if (IS_APPEND(inode) || IS_IMMUTABLE(inode)) {
> +			err = -EPERM;
> +			goto out_err;
> +		}
> +
> +		/*
> +		 * Reject non-regular files explicitly.  While
> standard VFS
> +		 * entry points (e.g. vfs_truncate) enforce S_ISREG,
> the caller
> +		 * notify_change() can be invoked directly by
> stackable
> +		 * filesystems, etc.
> +		 */
> +		if (!likely(S_ISREG(inode->i_mode))) {
> +			err = -EINVAL;
> +			goto out_err;
> +		}
> +
>  		inode_dio_wait(inode);
> +
> +		/*
> +		 * truncate_setsize() updates i_size and, upon size
> extension,
> +		 * zero-fills post-EOF page data if a block exists
> beyond the
> +		 * old EOF in the page.
> +		 */
>  		truncate_setsize(inode, iattr->ia_size);
> -		nilfs_truncate(inode);
> +
> +		if (iattr->ia_size < oldsize) {
> +			/*
> +			 * Zero the post-EOF portion of the boundary
> block
> +			 * (if exists) and truncate blocks beyond
> the new EOF.
> +			 */
> +			err = nilfs_truncate(inode);
> +		} else if (oldsize & (sb->s_blocksize - 1)) {
> +			/*
> +			 * Zero the unaligned post-EOF portion of
> the boundary
> +			 * block, which VFS skips when the extension
> stays
> +			 * within the same block or no block exists
> beyond
> +			 * the old EOF in the page.
> +			 */
> +			err = block_truncate_page(inode->i_mapping,
> oldsize,
> +						nilfs_get_block);
> +			if (likely(!err))
> +				nilfs_set_file_dirty(inode, 1);
> +		}
> +		if (unlikely(err)) {
> +			i_size_write(inode, oldsize);
> +			goto out_err;
> +		}
> +
> +		inode_set_mtime_to_ts(inode,
> inode_set_ctime_current(inode));
> +		if (IS_SYNC(inode))
> +			nilfs_set_transaction_flag(NILFS_TI_SYNC);
>  	}
>  
>  	setattr_copy(&nop_mnt_idmap, inode, iattr);
> diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
> index 28d35b2fa7f7..40dd42133dc6 100644
> --- a/fs/nilfs2/nilfs.h
> +++ b/fs/nilfs2/nilfs.h
> @@ -297,7 +297,6 @@ int nilfs_attach_btree_node_cache(struct inode
> *inode);
>  void nilfs_detach_btree_node_cache(struct inode *inode);
>  struct inode *nilfs_iget_for_shadow(struct inode *inode);
>  extern void nilfs_update_inode(struct inode *, struct buffer_head *,
> int);
> -extern void nilfs_truncate(struct inode *);
>  extern void nilfs_evict_inode(struct inode *);
>  extern int nilfs_setattr(const struct mnt_idmap *, struct dentry *,
>  			 struct iattr *);

Applied.

Thanks,
Slava.