FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Ubuntu > Ubuntu Kernel Team

 
 
LinkBack Thread Tools
 
Old 08-22-2012, 12:16 PM
Tim Gardner
 
Default ACK: eCryptfs: Initialize empty lower files when opening them

--
Tim Gardner tim.gardner@canonical.com

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 08-22-2012, 12:18 PM
Tim Gardner
 
Default ACK: eCryptfs: Initialize empty lower files when opening them

--
Tim Gardner tim.gardner@canonical.com

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 08-22-2012, 12:21 PM
Seth Forshee
 
Default ACK: eCryptfs: Initialize empty lower files when opening them

On Wed, Aug 22, 2012 at 11:44:35AM +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 | 71 ++++++++++++++++++++++++++---------------
> fs/ecryptfs/inode.c | 2 +-
> 3 files changed, 47 insertions(+), 27 deletions(-)
>
> diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
> index 4181136..f3a94b5 100644
> --- a/fs/ecryptfs/ecryptfs_kernel.h
> +++ b/fs/ecryptfs/ecryptfs_kernel.h
> @@ -630,6 +630,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 502b09f..669dd8d 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
> @@ -218,32 +260,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
");
> - 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
");
> - mutex_unlock(&crypt_stat->cs_mutex);
> - goto out_free;
> - }
> - rc = 0;
> - crypt_stat->flags &= ~(ECRYPTFS_I_SIZE_INITIALIZED
> - | 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%.16x] "
> "size: [0x%.16x]
", inode, inode->i_ino,
> i_size_read(inode));
> diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
> index 3c1dbc0..7d6574c 100644
> --- a/fs/ecryptfs/inode.c
> +++ b/fs/ecryptfs/inode.c
> @@ -173,7 +173,7 @@ static int grow_file(struct dentry *ecryptfs_dentry)
> *
> * 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

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 08-22-2012, 12:21 PM
Seth Forshee
 
Default ACK: eCryptfs: Initialize empty lower files when opening them

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
");
> - 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
");
> - 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]
", 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

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 

Thread Tools




All times are GMT. The time now is 10:26 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org