Ack: [PATCH 1/1] eCryptfs: Initialize empty lower files when opening them

[Seth Forshee]

On Wed, Aug 22, 2012 at 11:44:41AM +0100, Colin King wrote:
> From: Colin Ian King <colin.king at canonical.com>
> 
> Historically, eCryptfs has only initialized lower files in the
> ecryptfs_create() path. Lower file initialization is the act of writing
> the cryptographic metadata from the inode's crypt_stat to the header of
> the file. The ecryptfs_open() path already expects that metadata to be
> in the header of the file.
> 
> A number of users have reported empty lower files in beneath their
> eCryptfs mounts. Most of the causes for those empty files being left
> around have been addressed, but the presence of empty files causes
> problems due to the lack of proper cryptographic metadata.
> 
> To transparently solve this problem, this patch initializes empty lower
> files in the ecryptfs_open() error path. If the metadata is unreadable
> due to the lower inode size being 0, plaintext passthrough support is
> not in use, and the metadata is stored in the header of the file (as
> opposed to the user.ecryptfs extended attribute), the lower file will be
> initialized.
> 
> The number of nested conditionals in ecryptfs_open() was getting out of
> hand, so a helper function was created. To avoid the same nested
> conditional problem, the conditional logic was reversed inside of the
> helper function.
> 
> https://launchpad.net/bugs/911507
> 
> 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>
> (backport from upstream commit e3ccaa9761200952cc269b1f4b7d7bb77a5e071b)
> Signed-off-by: Colin Ian King <colin.king at canonical.com>
> ---
>  fs/ecryptfs/ecryptfs_kernel.h |    1 +
>  fs/ecryptfs/file.c            |   70 ++++++++++++++++++++++++++---------------
>  fs/ecryptfs/inode.c           |    2 +-
>  3 files changed, 47 insertions(+), 26 deletions(-)
> 
> diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
> index b920b28..2d624e0 100644
> --- a/fs/ecryptfs/ecryptfs_kernel.h
> +++ b/fs/ecryptfs/ecryptfs_kernel.h
> @@ -640,6 +640,7 @@ int ecryptfs_interpose(struct dentry *hidden_dentry,
>  		       struct dentry *this_dentry, struct super_block *sb,
>  		       u32 flags);
>  void ecryptfs_i_size_init(const char *page_virt, struct inode *inode);
> +int ecryptfs_initialize_file(struct dentry *ecryptfs_dentry);
>  int ecryptfs_lookup_and_interpose_lower(struct dentry *ecryptfs_dentry,
>  					struct dentry *lower_dentry,
>  					struct inode *ecryptfs_dir_inode);
> diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c
> index d30aac2..375dbb2 100644
> --- a/fs/ecryptfs/file.c
> +++ b/fs/ecryptfs/file.c
> @@ -141,6 +141,48 @@ out:
>  
>  struct kmem_cache *ecryptfs_file_info_cache;
>  
> +static int read_or_initialize_metadata(struct dentry *dentry)
> +{
> +	struct inode *inode = dentry->d_inode;
> +	struct ecryptfs_mount_crypt_stat *mount_crypt_stat;
> +	struct ecryptfs_crypt_stat *crypt_stat;
> +	int rc;
> +
> +	crypt_stat = &ecryptfs_inode_to_private(inode)->crypt_stat;
> +	mount_crypt_stat = &ecryptfs_superblock_to_private(
> +						inode->i_sb)->mount_crypt_stat;
> +	mutex_lock(&crypt_stat->cs_mutex);
> +
> +	if (crypt_stat->flags & ECRYPTFS_POLICY_APPLIED &&
> +	    crypt_stat->flags & ECRYPTFS_KEY_VALID) {
> +		rc = 0;
> +		goto out;
> +	}
> +
> +	rc = ecryptfs_read_metadata(dentry);
> +	if (!rc)
> +		goto out;
> +
> +	if (mount_crypt_stat->flags & ECRYPTFS_PLAINTEXT_PASSTHROUGH_ENABLED) {
> +		crypt_stat->flags &= ~(ECRYPTFS_I_SIZE_INITIALIZED
> +				       | ECRYPTFS_ENCRYPTED);
> +		rc = 0;
> +		goto out;
> +	}
> +
> +	if (!(mount_crypt_stat->flags & ECRYPTFS_XATTR_METADATA_ENABLED) &&
> +	    !i_size_read(ecryptfs_inode_to_lower(inode))) {
> +		rc = ecryptfs_initialize_file(dentry);
> +		if (!rc)
> +			goto out;
> +	}
> +
> +	rc = -EIO;
> +out:
> +	mutex_unlock(&crypt_stat->cs_mutex);
> +	return rc;
> +}
> +
>  /**
>   * ecryptfs_open
>   * @inode: inode speciying file to open
> @@ -217,31 +259,9 @@ static int ecryptfs_open(struct inode *inode, struct file *file)
>  		rc = 0;
>  		goto out;
>  	}
> -	mutex_lock(&crypt_stat->cs_mutex);
> -	if (!(crypt_stat->flags & ECRYPTFS_POLICY_APPLIED)
> -	    || !(crypt_stat->flags & ECRYPTFS_KEY_VALID)) {
> -		rc = ecryptfs_read_metadata(ecryptfs_dentry);
> -		if (rc) {
> -			ecryptfs_printk(KERN_DEBUG,
> -					"Valid headers not found\n");
> -			if (!(mount_crypt_stat->flags
> -			      & ECRYPTFS_PLAINTEXT_PASSTHROUGH_ENABLED)) {
> -				rc = -EIO;
> -				printk(KERN_WARNING "Either the lower file "
> -				       "is not in a valid eCryptfs format, "
> -				       "or the key could not be retrieved. "
> -				       "Plaintext passthrough mode is not "
> -				       "enabled; returning -EIO\n");
> -				mutex_unlock(&crypt_stat->cs_mutex);
> -				goto out_free;
> -			}
> -			rc = 0;
> -			crypt_stat->flags &= ~(ECRYPTFS_ENCRYPTED);
> -			mutex_unlock(&crypt_stat->cs_mutex);
> -			goto out;
> -		}
> -	}
> -	mutex_unlock(&crypt_stat->cs_mutex);
> +	rc = read_or_initialize_metadata(ecryptfs_dentry);
> +	if (rc)
> +		goto out_free;
>  	ecryptfs_printk(KERN_DEBUG, "inode w/ addr = [0x%p], i_ino = "
>  			"[0x%.16lx] size: [0x%.16llx]\n", inode, inode->i_ino,
>  			(unsigned long long)i_size_read(inode));
> diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
> index 5bb6e85..47d190e 100644
> --- a/fs/ecryptfs/inode.c
> +++ b/fs/ecryptfs/inode.c
> @@ -150,7 +150,7 @@ out:
>   *
>   * Returns zero on success
>   */
> -static int ecryptfs_initialize_file(struct dentry *ecryptfs_dentry)
> +int ecryptfs_initialize_file(struct dentry *ecryptfs_dentry)
>  {
>  	struct ecryptfs_crypt_stat *crypt_stat =
>  		&ecryptfs_inode_to_private(ecryptfs_dentry->d_inode)->crypt_stat;
> -- 
> 1.7.9.5
> 
> 
> -- 
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team