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

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

[Ryusuke Konishi]

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@gmail.com>
Closes: https://lore.kernel.org/all/47d7da4a-5029-4c56-a5ac-3e4c526cc569@gmail.com
Fixes: 05fe58fdc10d ("nilfs2: inode operations")
Signed-off-by: Ryusuke Konishi <konishi.ryusuke@gmail.com>
---
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 *);
-- 
2.53.0

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@gmail.com>
> Closes:
> https://lore.kernel.org/all/47d7da4a-5029-4c56-a5ac-3e4c526cc569@gmail.com
> Fixes: 05fe58fdc10d ("nilfs2: inode operations")
> Signed-off-by: Ryusuke Konishi <konishi.ryusuke@gmail.com>
> ---
> 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.