On 2012-06-12 19:30:47, Tyler Hicks wrote: > 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@xxxxxxxxxxxxx> > Cc: John Johansen <john.johansen@xxxxxxxxxxxxx> > Cc: Colin Ian King <colin.king@xxxxxxxxxxxxx> > --- > fs/ecryptfs/ecryptfs_kernel.h | 2 ++ > fs/ecryptfs/file.c | 69 +++++++++++++++++++++++++---------------- > fs/ecryptfs/inode.c | 4 +-- > 3 files changed, 47 insertions(+), 28 deletions(-) > > diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h > index 867b64c..56e3aa5 100644 > --- a/fs/ecryptfs/ecryptfs_kernel.h > +++ b/fs/ecryptfs/ecryptfs_kernel.h > @@ -568,6 +568,8 @@ struct ecryptfs_open_req { > struct inode *ecryptfs_get_inode(struct inode *lower_inode, > struct super_block *sb); > void ecryptfs_i_size_init(const char *page_virt, struct inode *inode); > +int ecryptfs_initialize_file(struct dentry *ecryptfs_dentry, > + struct inode *ecryptfs_inode); > int ecryptfs_decode_and_decrypt_filename(char **decrypted_name, > size_t *decrypted_name_size, > struct dentry *ecryptfs_dentry, > diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c > index 2b17f2f..cb8dd72 100644 > --- a/fs/ecryptfs/file.c > +++ b/fs/ecryptfs/file.c > @@ -161,6 +161,48 @@ static int ecryptfs_file_mmap(struct file *file, struct vm_area_struct *vma) > > 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, inode); > + if (!rc) > + goto out; > + } > + > + rc = -EIO; > +out: > + mutex_unlock(&crypt_stat->cs_mutex); > + return rc; > +} > + > /** > * ecryptfs_open > * @inode: inode speciying file to open > @@ -236,32 +278,7 @@ 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_put; > - } > - 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); I've discovered a problem with this patch. I didn't check the return value of read_or_initialize_metadata() here. Adding this snippet fixes the problem: if (rc) goto out_put; I'll push the corrected patch to the eCryptfs next branch. Tyler > 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 65efe5f..2d4143f 100644 > --- a/fs/ecryptfs/inode.c > +++ b/fs/ecryptfs/inode.c > @@ -227,8 +227,8 @@ out: > * > * Returns zero on success > */ > -static int ecryptfs_initialize_file(struct dentry *ecryptfs_dentry, > - struct inode *ecryptfs_inode) > +int ecryptfs_initialize_file(struct dentry *ecryptfs_dentry, > + struct inode *ecryptfs_inode) > { > struct ecryptfs_crypt_stat *crypt_stat = > &ecryptfs_inode_to_private(ecryptfs_inode)->crypt_stat; > -- > 1.7.9.5 > > -- > To unsubscribe from this list: send the line "unsubscribe ecryptfs" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html
Attachment:
signature.asc
Description: Digital signature
