* [PATCH] ubifs: remove redundant checks for encryption key
@ 2016-12-19 19:15 Eric Biggers
2016-12-19 19:59 ` Richard Weinberger
0 siblings, 1 reply; 3+ messages in thread
From: Eric Biggers @ 2016-12-19 19:15 UTC (permalink / raw)
To: linux-mtd
Cc: Richard Weinberger, Artem Bityutskiy, Adrian Hunter,
linux-fsdevel, Eric Biggers
From: Eric Biggers <ebiggers@google.com>
In several places, ubifs checked for an encryption key before creating a
file in an encrypted directory. This was redundant with
fscrypt_setup_filename() or ubifs_new_inode(), and in the case of
ubifs_link() it broke linking to special files. So remove the extra
checks.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
fs/ubifs/dir.c | 58 +++-------------------------------------------------------
1 file changed, 3 insertions(+), 55 deletions(-)
diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 9de9eaa..d346f1e 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -400,16 +400,6 @@ static int do_tmpfile(struct inode *dir, struct dentry *dentry,
dbg_gen("dent '%pd', mode %#hx in dir ino %lu",
dentry, mode, dir->i_ino);
- if (ubifs_crypt_is_encrypted(dir)) {
- err = fscrypt_get_encryption_info(dir);
- if (err)
- return err;
-
- if (!fscrypt_has_encryption_key(dir)) {
- return -EPERM;
- }
- }
-
err = fscrypt_setup_filename(dir, &dentry->d_name, 0, &nm);
if (err)
return err;
@@ -751,17 +741,9 @@ static int ubifs_link(struct dentry *old_dentry, struct inode *dir,
ubifs_assert(inode_is_locked(dir));
ubifs_assert(inode_is_locked(inode));
- if (ubifs_crypt_is_encrypted(dir)) {
- if (!fscrypt_has_permitted_context(dir, inode))
- return -EPERM;
-
- err = fscrypt_get_encryption_info(inode);
- if (err)
- return err;
-
- if (!fscrypt_has_encryption_key(inode))
- return -EPERM;
- }
+ if (ubifs_crypt_is_encrypted(dir) &&
+ !fscrypt_has_permitted_context(dir, inode))
+ return -EPERM;
err = fscrypt_setup_filename(dir, &dentry->d_name, 0, &nm);
if (err)
@@ -1010,17 +992,6 @@ static int ubifs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
if (err)
return err;
- if (ubifs_crypt_is_encrypted(dir)) {
- err = fscrypt_get_encryption_info(dir);
- if (err)
- goto out_budg;
-
- if (!fscrypt_has_encryption_key(dir)) {
- err = -EPERM;
- goto out_budg;
- }
- }
-
err = fscrypt_setup_filename(dir, &dentry->d_name, 0, &nm);
if (err)
goto out_budg;
@@ -1106,17 +1077,6 @@ static int ubifs_mknod(struct inode *dir, struct dentry *dentry,
return err;
}
- if (ubifs_crypt_is_encrypted(dir)) {
- err = fscrypt_get_encryption_info(dir);
- if (err)
- goto out_budg;
-
- if (!fscrypt_has_encryption_key(dir)) {
- err = -EPERM;
- goto out_budg;
- }
- }
-
err = fscrypt_setup_filename(dir, &dentry->d_name, 0, &nm);
if (err)
goto out_budg;
@@ -1241,18 +1201,6 @@ static int ubifs_symlink(struct inode *dir, struct dentry *dentry,
goto out_inode;
}
- err = fscrypt_get_encryption_info(inode);
- if (err) {
- kfree(sd);
- goto out_inode;
- }
-
- if (!fscrypt_has_encryption_key(inode)) {
- kfree(sd);
- err = -EPERM;
- goto out_inode;
- }
-
ostr.name = sd->encrypted_path;
ostr.len = disk_link.len;
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] ubifs: remove redundant checks for encryption key
2016-12-19 19:15 [PATCH] ubifs: remove redundant checks for encryption key Eric Biggers
@ 2016-12-19 19:59 ` Richard Weinberger
2016-12-19 20:24 ` Eric Biggers
0 siblings, 1 reply; 3+ messages in thread
From: Richard Weinberger @ 2016-12-19 19:59 UTC (permalink / raw)
To: Eric Biggers, linux-mtd
Cc: Artem Bityutskiy, Adrian Hunter, linux-fsdevel, Eric Biggers
On 19.12.2016 20:15, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> In several places, ubifs checked for an encryption key before creating a
> file in an encrypted directory. This was redundant with
> fscrypt_setup_filename() or ubifs_new_inode(), and in the case of
> ubifs_link() it broke linking to special files. So remove the extra
> checks.
Thanks for doing this. I assume same or similar changes were also needed
for f2fs and ext4 since I've duplicated the logic from them? :-)
Thanks,
//richard
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] ubifs: remove redundant checks for encryption key
2016-12-19 19:59 ` Richard Weinberger
@ 2016-12-19 20:24 ` Eric Biggers
0 siblings, 0 replies; 3+ messages in thread
From: Eric Biggers @ 2016-12-19 20:24 UTC (permalink / raw)
To: Richard Weinberger
Cc: linux-mtd, Artem Bityutskiy, Adrian Hunter, linux-fsdevel, Eric Biggers
On Mon, Dec 19, 2016 at 08:59:15PM +0100, Richard Weinberger wrote:
> On 19.12.2016 20:15, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> >
> > In several places, ubifs checked for an encryption key before creating a
> > file in an encrypted directory. This was redundant with
> > fscrypt_setup_filename() or ubifs_new_inode(), and in the case of
> > ubifs_link() it broke linking to special files. So remove the extra
> > checks.
>
> Thanks for doing this. I assume same or similar changes were also needed
> for f2fs and ext4 since I've duplicated the logic from them? :-)
>
Well all the filesystems are a little different, but I checked link, create,
mkdir, mknod, symlink, and tmpfile. UBIFS generally seemed to be the odd one
out with regards to having these extra checks:
ext4_link(): relies on check in fscrypt_setup_filename()
ext4_create(): relies on check in __ext4_new_inode()
ext4_mkdir(): relies on check in __ext4_new_inode()
ext4_mknod(): relies on check in __ext4_new_inode()
ext4_symlink(): relies on __ext4_new_inode() preloading encryption info
of new inode
ext4_tmpfile(): relies on check in __ext4_new_inode()
f2fs_link(): relies on check in fscrypt_setup_filename()
f2fs_create(): relies on check in fscrypt_setup_filename()
f2fs_mkdir(): relies on check in fscrypt_setup_filename()
f2fs_mknod(): relies on check in fscrypt_setup_filename()
f2fs_symlink(): has an explicit call to fscrypt_get_encryption_info() because
f2fs_new_inode() doesn't preload encryption info of new inode.
Check of fscrypt_has_encryption_key() appears unnecessary.
f2fs_tmpfile(): has an explicit load of encryption key with no check; it's
unclear whether this is sufficient, since it never calls
fscrypt_setup_filename()
So I am wondering why ext4 "preloads" encryption keys of new inodes but f2fs
doesn't, and whether f2fs_tmpfile() is doing the correct checks. But neither fs
had all the redundant checks this patch removes from ubifs.
Eric
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-12-19 20:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-19 19:15 [PATCH] ubifs: remove redundant checks for encryption key Eric Biggers
2016-12-19 19:59 ` Richard Weinberger
2016-12-19 20:24 ` Eric Biggers
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).