Skip to content

Commit

Permalink
fscrypto: require write access to mount to set encryption policy
Browse files Browse the repository at this point in the history
Since setting an encryption policy requires writing metadata to the
filesystem, it should be guarded by mnt_want_write/mnt_drop_write.
Otherwise, a user could cause a write to a frozen or readonly
filesystem.  This was handled correctly by f2fs but not by ext4.  Make
fscrypt_process_policy() handle it rather than relying on the filesystem
to get it right.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Cc: stable@vger.kernel.org # 4.1+; check fs/{ext4,f2fs}
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Acked-by: Jaegeuk Kim <jaegeuk@kernel.org>
  • Loading branch information
Eric Biggers authored and Theodore Ts'o committed Sep 10, 2016
1 parent 002ced4 commit ba63f23
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 25 deletions.
38 changes: 25 additions & 13 deletions fs/crypto/policy.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <linux/random.h>
#include <linux/string.h>
#include <linux/fscrypto.h>
#include <linux/mount.h>

static int inode_has_encryption_context(struct inode *inode)
{
Expand Down Expand Up @@ -92,31 +93,42 @@ static int create_encryption_context_from_policy(struct inode *inode,
return inode->i_sb->s_cop->set_context(inode, &ctx, sizeof(ctx), NULL);
}

int fscrypt_process_policy(struct inode *inode,
int fscrypt_process_policy(struct file *filp,
const struct fscrypt_policy *policy)
{
struct inode *inode = file_inode(filp);
int ret;

if (!inode_owner_or_capable(inode))
return -EACCES;

if (policy->version != 0)
return -EINVAL;

ret = mnt_want_write_file(filp);
if (ret)
return ret;

if (!inode_has_encryption_context(inode)) {
if (!S_ISDIR(inode->i_mode))
return -EINVAL;
if (!inode->i_sb->s_cop->empty_dir)
return -EOPNOTSUPP;
if (!inode->i_sb->s_cop->empty_dir(inode))
return -ENOTEMPTY;
return create_encryption_context_from_policy(inode, policy);
ret = -EINVAL;
else if (!inode->i_sb->s_cop->empty_dir)
ret = -EOPNOTSUPP;
else if (!inode->i_sb->s_cop->empty_dir(inode))
ret = -ENOTEMPTY;
else
ret = create_encryption_context_from_policy(inode,
policy);
} else if (!is_encryption_context_consistent_with_policy(inode,
policy)) {
printk(KERN_WARNING
"%s: Policy inconsistent with encryption context\n",
__func__);
ret = -EINVAL;
}

if (is_encryption_context_consistent_with_policy(inode, policy))
return 0;

printk(KERN_WARNING "%s: Policy inconsistent with encryption context\n",
__func__);
return -EINVAL;
mnt_drop_write_file(filp);
return ret;
}
EXPORT_SYMBOL(fscrypt_process_policy);

Expand Down
2 changes: 1 addition & 1 deletion fs/ext4/ioctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -776,7 +776,7 @@ long ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
(struct fscrypt_policy __user *)arg,
sizeof(policy)))
return -EFAULT;
return fscrypt_process_policy(inode, &policy);
return fscrypt_process_policy(filp, &policy);
#else
return -EOPNOTSUPP;
#endif
Expand Down
9 changes: 1 addition & 8 deletions fs/f2fs/file.c
Original file line number Diff line number Diff line change
Expand Up @@ -1757,21 +1757,14 @@ static int f2fs_ioc_set_encryption_policy(struct file *filp, unsigned long arg)
{
struct fscrypt_policy policy;
struct inode *inode = file_inode(filp);
int ret;

if (copy_from_user(&policy, (struct fscrypt_policy __user *)arg,
sizeof(policy)))
return -EFAULT;

ret = mnt_want_write_file(filp);
if (ret)
return ret;

f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);
ret = fscrypt_process_policy(inode, &policy);

mnt_drop_write_file(filp);
return ret;
return fscrypt_process_policy(filp, &policy);
}

static int f2fs_ioc_get_encryption_policy(struct file *filp, unsigned long arg)
Expand Down
5 changes: 2 additions & 3 deletions include/linux/fscrypto.h
Original file line number Diff line number Diff line change
Expand Up @@ -274,8 +274,7 @@ extern void fscrypt_restore_control_page(struct page *);
extern int fscrypt_zeroout_range(struct inode *, pgoff_t, sector_t,
unsigned int);
/* policy.c */
extern int fscrypt_process_policy(struct inode *,
const struct fscrypt_policy *);
extern int fscrypt_process_policy(struct file *, const struct fscrypt_policy *);
extern int fscrypt_get_policy(struct inode *, struct fscrypt_policy *);
extern int fscrypt_has_permitted_context(struct inode *, struct inode *);
extern int fscrypt_inherit_context(struct inode *, struct inode *,
Expand Down Expand Up @@ -345,7 +344,7 @@ static inline int fscrypt_notsupp_zeroout_range(struct inode *i, pgoff_t p,
}

/* policy.c */
static inline int fscrypt_notsupp_process_policy(struct inode *i,
static inline int fscrypt_notsupp_process_policy(struct file *f,
const struct fscrypt_policy *p)
{
return -EOPNOTSUPP;
Expand Down

0 comments on commit ba63f23

Please sign in to comment.