| Message ID | 1481829584-50218-3-git-send-email-ebiggers3@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 15.12.2016 20:19, Eric Biggers wrote: > From: Eric Biggers <ebiggers@google.com> > > Now that fscrypt_has_permitted_context() compares the fscrypt_context > rather than the fscrypt_info when needed, it is no longer necessary to > delay fscrypt_has_permitted_context() from ->lookup() to ->open() for > regular files, as introduced in commit ff978b09f973 ("ext4 crypto: move > context consistency check to ext4_file_open()"). Therefore the check in > ->open(), along with the dget_parent() hack, can be removed. > > It's also no longer necessary to check the file type before calling > fscrypt_has_permitted_context(). > > This patch makes these changes for both ext4 and f2fs. > > Signed-off-by: Eric Biggers <ebiggers@google.com> > --- > fs/ext4/file.c | 12 ------------ > fs/ext4/namei.c | 10 ++-------- > fs/f2fs/file.c | 15 +++++---------- > fs/f2fs/namei.c | 7 ++----- > 4 files changed, 9 insertions(+), 35 deletions(-) Can please also take care of UBIFS? :-) Thanks, //richard -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Dec 16, 2016 at 01:22:51PM +0100, Richard Weinberger wrote: > On 15.12.2016 20:19, Eric Biggers wrote: > > From: Eric Biggers <ebiggers@google.com> > > > > Now that fscrypt_has_permitted_context() compares the fscrypt_context > > rather than the fscrypt_info when needed, it is no longer necessary to > > delay fscrypt_has_permitted_context() from ->lookup() to ->open() for > > regular files, as introduced in commit ff978b09f973 ("ext4 crypto: move > > context consistency check to ext4_file_open()"). Therefore the check in > > ->open(), along with the dget_parent() hack, can be removed. > > > > It's also no longer necessary to check the file type before calling > > fscrypt_has_permitted_context(). > > > > This patch makes these changes for both ext4 and f2fs. > > > > Signed-off-by: Eric Biggers <ebiggers@google.com> > > --- > > fs/ext4/file.c | 12 ------------ > > fs/ext4/namei.c | 10 ++-------- > > fs/f2fs/file.c | 15 +++++---------- > > fs/f2fs/namei.c | 7 ++----- > > 4 files changed, 9 insertions(+), 35 deletions(-) > > Can please also take care of UBIFS? :-) > > Thanks, > //richard Yes, I see that UBIFS encryption just got merged yesterday, so I'll send a version that updates UBIFS too. And it seems the fscrypt_has_permitted_context() call in ubifs_lookup() is missing, so I'll add that too. I'm wondering if it would make more sense to do a separate patch for each filesystem? But in this case the filesystem changes are dependent on the prior patches to fs/crypto/, so they can't simply be sent through the per-filesystem trees unless each one merges in the fs/crypto/ changes too. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/ext4/file.c b/fs/ext4/file.c index b5f1844..2123cd8 100644 --- a/fs/ext4/file.c +++ b/fs/ext4/file.c @@ -398,7 +398,6 @@ static int ext4_file_open(struct inode * inode, struct file * filp) struct super_block *sb = inode->i_sb; struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb); struct vfsmount *mnt = filp->f_path.mnt; - struct dentry *dir; struct path path; char buf[64], *cp; int ret; @@ -443,17 +442,6 @@ static int ext4_file_open(struct inode * inode, struct file * filp) return -ENOKEY; } - dir = dget_parent(file_dentry(filp)); - if (ext4_encrypted_inode(d_inode(dir)) && - !fscrypt_has_permitted_context(d_inode(dir), inode)) { - ext4_warning(inode->i_sb, - "Inconsistent encryption contexts: %lu/%lu", - (unsigned long) d_inode(dir)->i_ino, - (unsigned long) inode->i_ino); - dput(dir); - return -EPERM; - } - dput(dir); /* * Set up the jbd2_inode if we are opening the inode for * writing and the journal is present diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c index eadba91..eb8b064 100644 --- a/fs/ext4/namei.c +++ b/fs/ext4/namei.c @@ -1612,17 +1612,11 @@ static struct dentry *ext4_lookup(struct inode *dir, struct dentry *dentry, unsi return ERR_PTR(-EFSCORRUPTED); } if (!IS_ERR(inode) && ext4_encrypted_inode(dir) && - (S_ISDIR(inode->i_mode) || S_ISLNK(inode->i_mode)) && !fscrypt_has_permitted_context(dir, inode)) { - int nokey = ext4_encrypted_inode(inode) && - !fscrypt_has_encryption_key(inode); - iput(inode); - if (nokey) - return ERR_PTR(-ENOKEY); ext4_warning(inode->i_sb, "Inconsistent encryption contexts: %lu/%lu", - (unsigned long) dir->i_ino, - (unsigned long) inode->i_ino); + dir->i_ino, inode->i_ino); + iput(inode); return ERR_PTR(-EPERM); } } diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c index 49f10dc..381d39b 100644 --- a/fs/f2fs/file.c +++ b/fs/f2fs/file.c @@ -443,23 +443,18 @@ static int f2fs_file_mmap(struct file *file, struct vm_area_struct *vma) static int f2fs_file_open(struct inode *inode, struct file *filp) { int ret = generic_file_open(inode, filp); - struct dentry *dir; - if (!ret && f2fs_encrypted_inode(inode)) { + if (ret) + return ret; + + if (f2fs_encrypted_inode(inode)) { ret = fscrypt_get_encryption_info(inode); if (ret) return -EACCES; if (!fscrypt_has_encryption_key(inode)) return -ENOKEY; } - dir = dget_parent(file_dentry(filp)); - if (f2fs_encrypted_inode(d_inode(dir)) && - !fscrypt_has_permitted_context(d_inode(dir), inode)) { - dput(dir); - return -EPERM; - } - dput(dir); - return ret; + return 0; } int truncate_data_blocks_range(struct dnode_of_data *dn, int count) diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c index db33b56..980f783 100644 --- a/fs/f2fs/namei.c +++ b/fs/f2fs/namei.c @@ -322,11 +322,8 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry, goto err_out; } if (!IS_ERR(inode) && f2fs_encrypted_inode(dir) && - (S_ISDIR(inode->i_mode) || S_ISLNK(inode->i_mode)) && - !fscrypt_has_permitted_context(dir, inode)) { - bool nokey = f2fs_encrypted_inode(inode) && - !fscrypt_has_encryption_key(inode); - err = nokey ? -ENOKEY : -EPERM; + !fscrypt_has_permitted_context(dir, inode)) { + err = -EPERM; goto err_out; } return d_splice_alias(inode, dentry);