Re: [PATCH 7/7] ecryptfs: call notify_change from truncate_upper

On 2026-03-31 17:37:28, Christoph Hellwig wrote:
> Keep all the truncation logic in one place by also moving the call to
> notify_change into truncate_upper.  Rename the resulting function to
> __ecryptfs_truncate as it deals with both the lower and upper inodes.
> 
> Signed-off-by: Christoph Hellwig <[email protected]>
> ---
>  fs/ecryptfs/inode.c | 61 +++++++++++++++++++++------------------------
>  1 file changed, 28 insertions(+), 33 deletions(-)
> 
> diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
> index c87ee3c6ecba..256beed0e47d 100644
> --- a/fs/ecryptfs/inode.c
> +++ b/fs/ecryptfs/inode.c
> @@ -707,7 +707,7 @@ upper_size_to_lower_size(struct ecryptfs_crypt_stat *crypt_stat,
>  }
>  
>  /**
> - * truncate_upper
> + * __ecryptfs_truncate
>   * @dentry: The ecryptfs layer dentry
>   * @lower_ia: Address of the lower inode's attributes
>   *
> @@ -721,9 +721,10 @@ upper_size_to_lower_size(struct ecryptfs_crypt_stat *crypt_stat,
>   *
>   * Returns zero on success; non-zero otherwise
>   */
> -static int truncate_upper(struct dentry *dentry, struct iattr *lower_ia)
> +static int __ecryptfs_truncate(struct dentry *dentry, struct iattr *lower_ia)

Could you update the function documentation, as well? This sentence is
no longer true:

 * ... If ATTR_SIZE is set in lower_ia->ia_valid upon return,       
 * the caller must use lower_ia in a call to notify_change() to perform         
 * the truncation of the lower inode.                         

The rest looks good.

Tyler

>  {
>  	struct inode *inode = d_inode(dentry);
> +	struct dentry *lower_dentry = ecryptfs_dentry_to_lower(dentry);
>  	struct ecryptfs_crypt_stat *crypt_stat;
>  	loff_t i_size = i_size_read(inode);
>  	loff_t lower_size_before_truncate;
> @@ -731,10 +732,8 @@ static int truncate_upper(struct dentry *dentry, struct iattr *lower_ia)
>  	size_t num_zeros;
>  	int rc;
>  
> -	if (unlikely(lower_ia->ia_size == i_size)) {
> -		lower_ia->ia_valid &= ~ATTR_SIZE;
> +	if (unlikely(lower_ia->ia_size == i_size))
>  		return 0;
> -	}
>  
>  	crypt_stat = &ecryptfs_inode_to_private(inode)->crypt_stat;
>  	lower_size_before_truncate =
> @@ -767,13 +766,12 @@ static int truncate_upper(struct dentry *dentry, struct iattr *lower_ia)
>  		 * new and of the file.
>  		 */
>  		rc = ecryptfs_write(inode, zero, lower_ia->ia_size - 1, 1);
> -		lower_ia->ia_valid &= ~ATTR_SIZE;
>  		goto out;
>  	}
>  
>  	if (!(crypt_stat->flags & ECRYPTFS_ENCRYPTED)) {
>  		truncate_setsize(inode, lower_ia->ia_size);
> -		goto out;
> +		goto set_size;
>  	}
>  
>  	/*
> @@ -803,10 +801,14 @@ static int truncate_upper(struct dentry *dentry, struct iattr *lower_ia)
>  	 * We are reducing the size of the ecryptfs file, and need to know if we
>  	 * need to reduce the size of the lower file.
>  	 */
> -	if (lower_size_after_truncate < lower_size_before_truncate)
> -		lower_ia->ia_size = lower_size_after_truncate;
> -	else
> -		lower_ia->ia_valid &= ~ATTR_SIZE;
> +	if (lower_size_after_truncate >= lower_size_before_truncate)
> +		goto out;
> +
> +	lower_ia->ia_size = lower_size_after_truncate;
> +set_size:
> +	inode_lock(d_inode(lower_dentry));
> +	rc = notify_change(&nop_mnt_idmap, lower_dentry, lower_ia, NULL);
> +	inode_unlock(d_inode(lower_dentry));
>  out:
>  	ecryptfs_put_lower_file(inode);
>  	return rc;
> @@ -828,18 +830,8 @@ int ecryptfs_truncate(struct dentry *dentry, loff_t new_length)
>  		.ia_valid	= ATTR_SIZE,
>  		.ia_size	= new_length,
>  	};
> -	int rc;
> -
> -	rc = truncate_upper(dentry, &lower_ia);
> -	if (!rc && lower_ia.ia_valid & ATTR_SIZE) {
> -		struct dentry *lower_dentry = ecryptfs_dentry_to_lower(dentry);
>  
> -		inode_lock(d_inode(lower_dentry));
> -		rc = notify_change(&nop_mnt_idmap, lower_dentry,
> -				   &lower_ia, NULL);
> -		inode_unlock(d_inode(lower_dentry));
> -	}
> -	return rc;
> +	return __ecryptfs_truncate(dentry, &lower_ia);
>  }
>  
>  static int
> @@ -867,7 +859,6 @@ static int ecryptfs_setattr(struct mnt_idmap *idmap,
>  			    struct dentry *dentry, struct iattr *ia)
>  {
>  	struct inode *inode = d_inode(dentry);
> -	struct dentry *lower_dentry = ecryptfs_dentry_to_lower(dentry);
>  	struct inode *lower_inode = ecryptfs_inode_to_lower(inode);
>  	struct iattr lower_ia;
>  	struct ecryptfs_crypt_stat *crypt_stat;
> @@ -918,13 +909,6 @@ static int ecryptfs_setattr(struct mnt_idmap *idmap,
>  		goto out;
>  
>  	memcpy(&lower_ia, ia, sizeof(lower_ia));
> -	if (ia->ia_valid & ATTR_FILE)
> -		lower_ia.ia_file = ecryptfs_file_to_lower(ia->ia_file);
> -	if (ia->ia_valid & ATTR_SIZE) {
> -		rc = truncate_upper(dentry, &lower_ia);
> -		if (rc < 0)
> -			goto out;
> -	}
>  
>  	/*
>  	 * mode change is for clearing setuid/setgid bits. Allow lower fs
> @@ -933,9 +917,20 @@ static int ecryptfs_setattr(struct mnt_idmap *idmap,
>  	if (lower_ia.ia_valid & (ATTR_KILL_SUID | ATTR_KILL_SGID))
>  		lower_ia.ia_valid &= ~ATTR_MODE;
>  
> -	inode_lock(d_inode(lower_dentry));
> -	rc = notify_change(&nop_mnt_idmap, lower_dentry, &lower_ia, NULL);
> -	inode_unlock(d_inode(lower_dentry));
> +	if (ia->ia_valid & ATTR_SIZE) {
> +		if (ia->ia_valid & ATTR_FILE)
> +			lower_ia.ia_file = ecryptfs_file_to_lower(ia->ia_file);
> +		rc = __ecryptfs_truncate(dentry, &lower_ia);
> +		if (rc < 0)
> +			goto out;
> +	} else {
> +		struct dentry *lower_dentry = ecryptfs_dentry_to_lower(dentry);
> +
> +		inode_lock(d_inode(lower_dentry));
> +		rc = notify_change(&nop_mnt_idmap, lower_dentry, &lower_ia,
> +			NULL);
> +		inode_unlock(d_inode(lower_dentry));
> +	}
>  out:
>  	fsstack_copy_attr_all(inode, lower_inode);
>  	return rc;
> -- 
> 2.47.3
>