diff mbox

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

Message ID 1345632281-1859-2-git-send-email-colin.king@canonical.com
State New
Headers show

Commit Message

Colin Ian King Aug. 22, 2012, 10:44 a.m. UTC 
From: Colin Ian King <colin.king@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@canonical.com>
Cc: John Johansen <john.johansen@canonical.com>
Cc: Colin Ian King <colin.king@canonical.com>
(backport from upstream commit e3ccaa9761200952cc269b1f4b7d7bb77a5e071b)
Signed-off-by: Colin Ian King <colin.king@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(-)

Comments

Tim Gardner Aug. 22, 2012, 12:18 p.m. UTC | #1
Seth Forshee Aug. 22, 2012, 12:21 p.m. UTC | #2 
On Wed, Aug 22, 2012 at 11:44:41AM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@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@canonical.com>
> Cc: John Johansen <john.johansen@canonical.com>
> Cc: Colin Ian King <colin.king@canonical.com>
> (backport from upstream commit e3ccaa9761200952cc269b1f4b7d7bb77a5e071b)
> Signed-off-by: Colin Ian King <colin.king@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@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff mbox

Patch

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;