From da025e063d902e7ae8c838abcc39c8f285699d0f Mon Sep 17 00:00:00 2001 From: Tyler Hicks Date: Mon, 26 Mar 2012 11:43:59 -0500 Subject: [PATCH] eCryptfs: Ensure i_size is set in ecryptfs_getattr() If the eCryptfs inode size was not set in the ecryptfs_lookup() path, we need to force another partial metadata read in ecryptfs_getattr() to be sure that userspace gets a valid st_size. If the read of the eCryptfs metadata during ecryptfs_lookup() was interrupted, it could result in an inode with an i_size incorrectly equal to that of the lower file (the ECRYPTFS_I_SIZE_INITIALIZED flag would also not be set). ecryptfs_getattr() would then fill the kstat using the bad i_size. If the file was then opened, a full parsing of the metadata occurred and i_size would be properly set. But the amount of readable data would not match what was reported earlier in the flawed stat. The fix is to check for the ECRYPTFS_I_SIZE_INITIALIZED flag and if it is not set, make sure that the lower file is an eCryptfs-encrypted file and then read the decrypted i_size from the metdata. ecryptfs_i_size_read() was changed to return errors if there was a problem reading the decrypted i_size from the metadata. It previously ignored those errors because ecryptfs_lookup_interpose() was the only caller and it needed to ignore the errors so that non-eCryptfs files in the lower filesystem could be looked up. The ecryptfs_read_and_validate_{header,xattr}_region() functions were changed to return 1 if the metadata marker couldn't be validated but plaintext passthrough was enabled. https://launchpad.net/bugs/842647 Signed-off-by: Tyler Hicks --- fs/ecryptfs/crypto.c | 39 +++++++++++++++++++++++++++++++-------- fs/ecryptfs/inode.c | 39 ++++++++++++++++++++++++++++----------- 2 files changed, 59 insertions(+), 19 deletions(-) diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c index ea99312..db303f4 100644 --- a/fs/ecryptfs/crypto.c +++ b/fs/ecryptfs/crypto.c @@ -1165,16 +1165,27 @@ int ecryptfs_read_and_validate_header_region(struct inode *inode) { u8 file_size[ECRYPTFS_SIZE_AND_MARKER_BYTES]; u8 *marker = file_size + ECRYPTFS_FILE_SIZE_BYTES; + struct ecryptfs_crypt_stat *crypt_stat; + struct ecryptfs_mount_crypt_stat *mount_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; + rc = ecryptfs_read_lower(file_size, 0, ECRYPTFS_SIZE_AND_MARKER_BYTES, inode); if (rc < ECRYPTFS_SIZE_AND_MARKER_BYTES) return rc >= 0 ? -EINVAL : rc; + rc = ecryptfs_validate_marker(marker); - if (!rc) - ecryptfs_i_size_init(file_size, inode); - return rc; + if (rc) + return (mount_crypt_stat->flags + & ECRYPTFS_PLAINTEXT_PASSTHROUGH_ENABLED) ? 1 : rc; + + ecryptfs_i_size_init(file_size, inode); + crypt_stat->flags |= ECRYPTFS_I_SIZE_INITIALIZED; + return 0; } void @@ -1421,7 +1432,6 @@ void ecryptfs_i_size_init(const char *page_virt, struct inode *inode) } else file_size = get_unaligned_be64(page_virt); i_size_write(inode, (loff_t)file_size); - crypt_stat->flags |= ECRYPTFS_I_SIZE_INITIALIZED; } /** @@ -1452,8 +1462,10 @@ static int ecryptfs_read_headers_virt(char *page_virt, rc = ecryptfs_validate_marker(page_virt + offset); if (rc) goto out; - if (!(crypt_stat->flags & ECRYPTFS_I_SIZE_INITIALIZED)) + if (!(crypt_stat->flags & ECRYPTFS_I_SIZE_INITIALIZED)) { ecryptfs_i_size_init(page_virt, ecryptfs_dentry->d_inode); + crypt_stat->flags |= ECRYPTFS_I_SIZE_INITIALIZED; + } offset += MAGIC_ECRYPTFS_MARKER_SIZE_BYTES; rc = ecryptfs_process_flags(crypt_stat, (page_virt + offset), &bytes_read); @@ -1523,17 +1535,28 @@ int ecryptfs_read_and_validate_xattr_region(struct dentry *dentry, { u8 file_size[ECRYPTFS_SIZE_AND_MARKER_BYTES]; u8 *marker = file_size + ECRYPTFS_FILE_SIZE_BYTES; + struct ecryptfs_crypt_stat *crypt_stat; + struct ecryptfs_mount_crypt_stat *mount_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; + rc = ecryptfs_getxattr_lower(ecryptfs_dentry_to_lower(dentry), ECRYPTFS_XATTR_NAME, file_size, ECRYPTFS_SIZE_AND_MARKER_BYTES); if (rc < ECRYPTFS_SIZE_AND_MARKER_BYTES) return rc >= 0 ? -EINVAL : rc; + rc = ecryptfs_validate_marker(marker); - if (!rc) - ecryptfs_i_size_init(file_size, inode); - return rc; + if (rc) + return (mount_crypt_stat->flags + & ECRYPTFS_PLAINTEXT_PASSTHROUGH_ENABLED) ? 1 : rc; + + ecryptfs_i_size_init(file_size, inode); + crypt_stat->flags |= ECRYPTFS_I_SIZE_INITIALIZED; + return 0; } /** diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c index ab35b11..97f3170 100644 --- a/fs/ecryptfs/inode.c +++ b/fs/ecryptfs/inode.c @@ -298,12 +298,17 @@ static int ecryptfs_i_size_read(struct dentry *dentry, struct inode *inode) rc = ecryptfs_read_and_validate_header_region(inode); ecryptfs_put_lower_file(inode); if (rc) { + int header_rc = rc; + rc = ecryptfs_read_and_validate_xattr_region(dentry, inode); if (!rc) crypt_stat->flags |= ECRYPTFS_METADATA_IN_XATTR; + else if (rc == 1 || header_rc == 1) + crypt_stat->flags |= ECRYPTFS_I_SIZE_INITIALIZED; + else if (rc < 0) + return -EINVAL; } - /* Must return 0 to allow non-eCryptfs files to be looked up, too */ return 0; } @@ -349,11 +354,8 @@ static int ecryptfs_lookup_interpose(struct dentry *dentry, return PTR_ERR(inode); } if (S_ISREG(inode->i_mode)) { - rc = ecryptfs_i_size_read(dentry, inode); - if (rc) { - make_bad_inode(inode); - return rc; - } + /* Errors ignored so non-eCryptfs files can be looked up, too */ + ecryptfs_i_size_read(dentry, inode); } if (inode->i_state & I_NEW) @@ -987,8 +989,8 @@ static int ecryptfs_setattr(struct dentry *dentry, struct iattr *ia) goto out; } rc = 0; - crypt_stat->flags &= ~(ECRYPTFS_I_SIZE_INITIALIZED - | ECRYPTFS_ENCRYPTED); + crypt_stat->flags &= ~ECRYPTFS_ENCRYPTED; + crypt_stat->flags |= ECRYPTFS_I_SIZE_INITIALIZED; } } mutex_unlock(&crypt_stat->cs_mutex); @@ -1057,15 +1059,30 @@ int ecryptfs_getattr_link(struct vfsmount *mnt, struct dentry *dentry, int ecryptfs_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat) { + struct inode *inode = dentry->d_inode; struct kstat lower_stat; int rc; + if (S_ISREG(inode->i_mode)) { + struct ecryptfs_crypt_stat *crypt_stat; + + crypt_stat = &ecryptfs_inode_to_private(inode)->crypt_stat; + mutex_lock(&crypt_stat->cs_mutex); + if (!(crypt_stat->flags & ECRYPTFS_I_SIZE_INITIALIZED)) { + rc = ecryptfs_i_size_read(dentry, inode); + if (rc) { + mutex_unlock(&crypt_stat->cs_mutex); + return rc; + } + } + mutex_unlock(&crypt_stat->cs_mutex); + } + rc = vfs_getattr(ecryptfs_dentry_to_lower_mnt(dentry), ecryptfs_dentry_to_lower(dentry), &lower_stat); if (!rc) { - fsstack_copy_attr_all(dentry->d_inode, - ecryptfs_inode_to_lower(dentry->d_inode)); - generic_fillattr(dentry->d_inode, stat); + fsstack_copy_attr_all(inode, ecryptfs_inode_to_lower(inode)); + generic_fillattr(inode, stat); stat->blocks = lower_stat.blocks; } return rc; -- 1.7.9.1