[ACK/CMT] Natty SRU: eCryptfs: Handle failed metadata read in lookup

Stefan Bader stefan.bader at canonical.com
Wed Jul 20 09:02:40 UTC 2011


On 20.07.2011 02:01, Tim Gardner wrote:
> The following changes since commit 2fbb6e80e5b7e9e31a1a81d96c5a39a5a2cc070d:
>   Keng-Yu Lin (1):
>         UBUNTU: SAUCE: Input: ALPS - Enable Intellimouse mode for Lenovo Zhaoyang E47
> 
> are available in the git repository at:
> 
>   git://kernel.ubuntu.com/rtg/ubuntu-natty.git ecryptfs-lp509180
> 
> Tim Gardner (1):
>       eCryptfs: Handle failed metadata read in lookup
> 
>  fs/ecryptfs/crypto.c          |   21 +++++++++++++++++++++
>  fs/ecryptfs/ecryptfs_kernel.h |    2 ++
>  fs/ecryptfs/file.c            |    3 ++-
>  fs/ecryptfs/inode.c           |   18 +++---------------
>  4 files changed, 28 insertions(+), 16 deletions(-)
> 
> From 9a2275b475ca346baba6445b2b737c61d9564afd Mon Sep 17 00:00:00 2001
> From: Tim Gardner <tim.gardner at canonical.com>
> Date: Fri, 15 Jul 2011 10:49:42 -0600
> Subject: [PATCH] eCryptfs: Handle failed metadata read in lookup
> 
> When failing to read the lower file's crypto metadata during a lookup,
> eCryptfs must continue on without throwing an error. For example, there
> may be a plaintext file in the lower mount point that the user wants to
> delete through the eCryptfs mount.
> 
> If an error is encountered while reading the metadata in lookup(), the
> eCryptfs inode's size could be incorrect. We must be sure to reread the
> plaintext inode size from the metadata when performing an open() or
> setattr(). The metadata is already being read in those paths, so this
> adds minimal performance overhead.
> 
> This patch introduces a flag which will track whether or not the
> plaintext inode size has been read so that an incorrect i_size can be
> fixed in the open() or setattr() paths.
> 
> https://bugs.launchpad.net/bugs/509180
> 
> Cc: <stable at kernel.org>
> Signed-off-by: Tyler Hicks <tyhicks at linux.vnet.ibm.com>
> 

BugLink: http://bugs.launchpad.net/bugs/509180

> (backported from 3aeb86ea4cd15f728147a3bd5469a205ada8c767)
> Signed-off-by: Tim Gardner <tim.gardner at canonical.com>
> ---
>  fs/ecryptfs/crypto.c          |   21 +++++++++++++++++++++
>  fs/ecryptfs/ecryptfs_kernel.h |    2 ++
>  fs/ecryptfs/file.c            |    3 ++-
>  fs/ecryptfs/inode.c           |   18 +++---------------
>  4 files changed, 28 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
> index bfd8b68..97b0a86 100644
> --- a/fs/ecryptfs/crypto.c
> +++ b/fs/ecryptfs/crypto.c
> @@ -1452,6 +1452,25 @@ static void set_default_header_data(struct ecryptfs_crypt_stat *crypt_stat)
>  	crypt_stat->metadata_size = ECRYPTFS_MINIMUM_HEADER_EXTENT_SIZE;
>  }
>  
> +void ecryptfs_i_size_init(const char *page_virt, struct inode *inode)
> +{
> +	struct ecryptfs_mount_crypt_stat *mount_crypt_stat;
> +	struct ecryptfs_crypt_stat *crypt_stat;
> +	u64 file_size;
> +
> +	crypt_stat = &ecryptfs_inode_to_private(inode)->crypt_stat;
> +	mount_crypt_stat =
> +		&ecryptfs_superblock_to_private(inode->i_sb)->mount_crypt_stat;
> +	if (mount_crypt_stat->flags & ECRYPTFS_ENCRYPTED_VIEW_ENABLED) {
> +		file_size = i_size_read(ecryptfs_inode_to_lower(inode));
> +		if (crypt_stat->flags & ECRYPTFS_METADATA_IN_XATTR)
> +			file_size += crypt_stat->metadata_size;
> +	} else
> +		file_size = get_unaligned_be64(page_virt);
> +	i_size_write(inode, (loff_t)file_size);
> +	crypt_stat->flags |= ECRYPTFS_I_SIZE_INITIALIZED;
> +}
> +
>  /**
>   * ecryptfs_read_headers_virt
>   * @page_virt: The virtual address into which to read the headers
> @@ -1482,6 +1501,8 @@ static int ecryptfs_read_headers_virt(char *page_virt,
>  		rc = -EINVAL;
>  		goto out;
>  	}
> +	if (!(crypt_stat->flags & ECRYPTFS_I_SIZE_INITIALIZED))
> +		ecryptfs_i_size_init(page_virt, ecryptfs_dentry->d_inode);
>  	offset += MAGIC_ECRYPTFS_MARKER_SIZE_BYTES;
>  	rc = ecryptfs_process_flags(crypt_stat, (page_virt + offset),
>  				    &bytes_read);
> diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
> index e007534..40efaa7 100644
> --- a/fs/ecryptfs/ecryptfs_kernel.h
> +++ b/fs/ecryptfs/ecryptfs_kernel.h
> @@ -270,6 +270,7 @@ struct ecryptfs_crypt_stat {
>  #define ECRYPTFS_ENCFN_USE_MOUNT_FNEK 0x00001000
>  #define ECRYPTFS_ENCFN_USE_FEK        0x00002000
>  #define ECRYPTFS_UNLINK_SIGS	      0x00004000
> +#define ECRYPTFS_I_SIZE_INITIALIZED   0x00008000
>  	u32 flags;
>  	unsigned int file_version;
>  	size_t iv_bytes;
> @@ -630,6 +631,7 @@ struct ecryptfs_open_req {
>  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_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 7d1050e..d30aac2 100644
> --- a/fs/ecryptfs/file.c
> +++ b/fs/ecryptfs/file.c
> @@ -211,7 +211,8 @@ static int ecryptfs_open(struct inode *inode, struct file *file)
>  	if (S_ISDIR(ecryptfs_dentry->d_inode->i_mode)) {
>  		ecryptfs_printk(KERN_DEBUG, "This is a directory\n");
>  		mutex_lock(&crypt_stat->cs_mutex);
> -		crypt_stat->flags &= ~(ECRYPTFS_ENCRYPTED);
> +		crypt_stat->flags &= ~(ECRYPTFS_I_SIZE_INITIALIZED
> +					| ECRYPTFS_ENCRYPTED);
>  		mutex_unlock(&crypt_stat->cs_mutex);
>  		rc = 0;
>  		goto out;
> diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
> index b592938..274d573 100644
> --- a/fs/ecryptfs/inode.c
> +++ b/fs/ecryptfs/inode.c
> @@ -250,10 +250,8 @@ int ecryptfs_lookup_and_interpose_lower(struct dentry *ecryptfs_dentry,
>  	struct dentry *lower_dir_dentry;
>  	struct vfsmount *lower_mnt;
>  	struct inode *lower_inode;
> -	struct ecryptfs_mount_crypt_stat *mount_crypt_stat;
>  	struct ecryptfs_crypt_stat *crypt_stat;
>  	char *page_virt = NULL;
> -	u64 file_size;
>  	int rc = 0;
>  
>  	lower_dir_dentry = lower_dentry->d_parent;
> @@ -326,18 +324,7 @@ int ecryptfs_lookup_and_interpose_lower(struct dentry *ecryptfs_dentry,
>  		}
>  		crypt_stat->flags |= ECRYPTFS_METADATA_IN_XATTR;
>  	}
> -	mount_crypt_stat = &ecryptfs_superblock_to_private(
> -		ecryptfs_dentry->d_sb)->mount_crypt_stat;
> -	if (mount_crypt_stat->flags & ECRYPTFS_ENCRYPTED_VIEW_ENABLED) {
> -		if (crypt_stat->flags & ECRYPTFS_METADATA_IN_XATTR)
> -			file_size = (crypt_stat->metadata_size
> -				     + i_size_read(lower_dentry->d_inode));
> -		else
> -			file_size = i_size_read(lower_dentry->d_inode);
> -	} else {
> -		file_size = get_unaligned_be64(page_virt);
> -	}
> -	i_size_write(ecryptfs_dentry->d_inode, (loff_t)file_size);
> +	ecryptfs_i_size_init(page_virt, ecryptfs_dentry->d_inode);
>  out_free_kmem:
>  	kmem_cache_free(ecryptfs_header_cache_2, page_virt);
>  	goto out;
> @@ -951,7 +938,8 @@ static int ecryptfs_setattr(struct dentry *dentry, struct iattr *ia)
>  				goto out;
>  			}
>  			rc = 0;
> -			crypt_stat->flags &= ~(ECRYPTFS_ENCRYPTED);
> +			crypt_stat->flags &= ~(ECRYPTFS_I_SIZE_INITIALIZED
> +						| ECRYPTFS_ENCRYPTED);
>  		}
>  	}
>  	mutex_unlock(&crypt_stat->cs_mutex);

Acked-by: Stefan Bader <stefan.bader at canonical.com>




More information about the kernel-team mailing list