[Precise SRU PATCH] eCryptfs: Unlink lower inode when ecryptfs_create() fails

[Stefan Bader]

I am having dejavus... and not yet had any beers...

On 07.08.2012 18:40, Tim Gardner wrote:
> From: Tyler Hicks <tyhicks at canonical.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/872905
> 
> ecryptfs_create() creates a lower inode, allocates an eCryptfs inode,
> initializes the eCryptfs inode and cryptographic metadata attached to
> the inode, and then writes the metadata to the header of the file.
> 
> If an error was to occur after the lower inode was created, an empty
> lower file would be left in the lower filesystem. This is a problem
> because ecryptfs_open() refuses to open any lower files which do not
> have the appropriate metadata in the file header.
> 
> This patch properly unlinks the lower inode when an error occurs in the
> later stages of ecryptfs_create(), reducing the chance that an empty
> lower file will be left in the lower filesystem.
> 
> https://launchpad.net/bugs/872905
> 
> Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
> Cc: John Johansen <john.johansen at canonical.com>
> Cc: Colin Ian King <colin.king at canonical.com>
> (cherry picked from commit 8bc2d3cf612994a960c2e8eaea37f6676f67082a)
> 
> Signed-off-by: Tim Gardner <tim.gardner at canonical.com>
> ---
>  fs/ecryptfs/inode.c |   55 ++++++++++++++++++++++++++++++---------------------
>  1 file changed, 32 insertions(+), 23 deletions(-)
> 
> diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
> index af11098..06f55bc 100644
> --- a/fs/ecryptfs/inode.c
> +++ b/fs/ecryptfs/inode.c
> @@ -143,6 +143,31 @@ static int ecryptfs_interpose(struct dentry *lower_dentry,
>  	return 0;
>  }
>  
> +static int ecryptfs_do_unlink(struct inode *dir, struct dentry *dentry,
> +			      struct inode *inode)
> +{
> +	struct dentry *lower_dentry = ecryptfs_dentry_to_lower(dentry);
> +	struct inode *lower_dir_inode = ecryptfs_inode_to_lower(dir);
> +	struct dentry *lower_dir_dentry;
> +	int rc;
> +
> +	dget(lower_dentry);
> +	lower_dir_dentry = lock_parent(lower_dentry);
> +	rc = vfs_unlink(lower_dir_inode, lower_dentry);
> +	if (rc) {
> +		printk(KERN_ERR "Error in vfs_unlink; rc = [%d]\n", rc);
> +		goto out_unlock;
> +	}
> +	fsstack_copy_attr_times(dir, lower_dir_inode);
> +	set_nlink(inode, ecryptfs_inode_to_lower(inode)->i_nlink);
> +	inode->i_ctime = dir->i_ctime;
> +	d_drop(dentry);
> +out_unlock:
> +	unlock_dir(lower_dir_dentry);
> +	dput(lower_dentry);
> +	return rc;
> +}
> +
>  /**
>   * ecryptfs_create_underlying_file
>   * @lower_dir_inode: inode of the parent in the lower fs of the new file
> @@ -201,8 +226,10 @@ ecryptfs_do_create(struct inode *directory_inode,
>  	}
>  	inode = __ecryptfs_get_inode(lower_dentry->d_inode,
>  				     directory_inode->i_sb);
> -	if (IS_ERR(inode))
> +	if (IS_ERR(inode)) {
> +		vfs_unlink(lower_dir_dentry->d_inode, lower_dentry);
>  		goto out_lock;
> +	}
>  	fsstack_copy_attr_times(directory_inode, lower_dir_dentry->d_inode);
>  	fsstack_copy_inode_size(directory_inode, lower_dir_dentry->d_inode);
>  out_lock:
> @@ -284,7 +311,9 @@ ecryptfs_create(struct inode *directory_inode, struct dentry *ecryptfs_dentry,
>  	 * that this on disk file is prepared to be an ecryptfs file */
>  	rc = ecryptfs_initialize_file(ecryptfs_dentry, ecryptfs_inode);
>  	if (rc) {
> -		drop_nlink(ecryptfs_inode);
> +		ecryptfs_do_unlink(directory_inode, ecryptfs_dentry,
> +				   ecryptfs_inode);
> +		make_bad_inode(ecryptfs_inode);
>  		unlock_new_inode(ecryptfs_inode);
>  		iput(ecryptfs_inode);
>  		goto out;
> @@ -496,27 +525,7 @@ out_lock:
>  
>  static int ecryptfs_unlink(struct inode *dir, struct dentry *dentry)
>  {
> -	int rc = 0;
> -	struct dentry *lower_dentry = ecryptfs_dentry_to_lower(dentry);
> -	struct inode *lower_dir_inode = ecryptfs_inode_to_lower(dir);
> -	struct dentry *lower_dir_dentry;
> -
> -	dget(lower_dentry);
> -	lower_dir_dentry = lock_parent(lower_dentry);
> -	rc = vfs_unlink(lower_dir_inode, lower_dentry);
> -	if (rc) {
> -		printk(KERN_ERR "Error in vfs_unlink; rc = [%d]\n", rc);
> -		goto out_unlock;
> -	}
> -	fsstack_copy_attr_times(dir, lower_dir_inode);
> -	set_nlink(dentry->d_inode,
> -		  ecryptfs_inode_to_lower(dentry->d_inode)->i_nlink);
> -	dentry->d_inode->i_ctime = dir->i_ctime;
> -	d_drop(dentry);
> -out_unlock:
> -	unlock_dir(lower_dir_dentry);
> -	dput(lower_dentry);
> -	return rc;
> +	return ecryptfs_do_unlink(dir, dentry, dentry->d_inode);
>  }
>  
>  static int ecryptfs_symlink(struct inode *dir, struct dentry *dentry,
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 900 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20120807/c7ac75bd/attachment.sig>