fscrypt-move-ioctl-processing-more-fully-into-common-code

   1 fscrypto: move ioctl processing more fully into common code
   2
   3 From: Eric Biggers <ebiggers@google.com>
   4
   5 Multiple bugs were recently fixed in the "set encryption policy" ioctl.
   6 To make it clear that fscrypt_process_policy() and fscrypt_get_policy()
   7 implement ioctls and therefore their implementations must take standard
   8 security and correctness precautions, rename them to
   9 fscrypt_ioctl_set_policy() and fscrypt_ioctl_get_policy().  Make the
  10 latter take in a struct file * to make it consistent with the former.
  11
  12 Signed-off-by: Eric Biggers <ebiggers@google.com>
  13 Signed-off-by: Theodore Ts'o <tytso@mit.edu>
  14 ---
  15  fs/crypto/policy.c       | 34 +++++++++++++++++++++-------------
  16  fs/ext4/ext4.h           |  4 ++--
  17  fs/ext4/ioctl.c          | 34 +++++-----------------------------
  18  fs/f2fs/f2fs.h           |  4 ++--
  19  fs/f2fs/file.c           | 19 ++-----------------
  20  include/linux/fscrypto.h | 12 ++++++------
  21  6 files changed, 38 insertions(+), 69 deletions(-)
  22
  23 diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
  24 index 6865663aac69..b96a10e3cf78 100644
  25 --- a/fs/crypto/policy.c
  26 +++ b/fs/crypto/policy.c
  27 @@ -93,16 +93,19 @@ static int create_encryption_context_from_policy(struct inode *inode,
  28         return inode->i_sb->s_cop->set_context(inode, &ctx, sizeof(ctx), NULL);
  29  }
  30
  31 -int fscrypt_process_policy(struct file *filp,
  32 -                               const struct fscrypt_policy *policy)
  33 +int fscrypt_ioctl_set_policy(struct file *filp, const void __user *arg)
  34  {
  35 +       struct fscrypt_policy policy;
  36         struct inode *inode = file_inode(filp);
  37         int ret;
  38
  39 +       if (copy_from_user(&policy, arg, sizeof(policy)))
  40 +               return -EFAULT;
  41 +
  42         if (!inode_owner_or_capable(inode))
  43                 return -EACCES;
  44
  45 -       if (policy->version != 0)
  46 +       if (policy.version != 0)
  47                 return -EINVAL;
  48
  49         ret = mnt_want_write_file(filp);
  50 @@ -120,9 +123,9 @@ int fscrypt_process_policy(struct file *filp,
  51                         ret = -ENOTEMPTY;
  52                 else
  53                         ret = create_encryption_context_from_policy(inode,
  54 -                                                                   policy);
  55 +                                                                   &policy);
  56         } else if (!is_encryption_context_consistent_with_policy(inode,
  57 -                                                                policy)) {
  58 +                                                                &policy)) {
  59                 printk(KERN_WARNING
  60                        "%s: Policy inconsistent with encryption context\n",
  61                        __func__);
  62 @@ -134,11 +137,13 @@ int fscrypt_process_policy(struct file *filp,
  63         mnt_drop_write_file(filp);
  64         return ret;
  65  }
  66 -EXPORT_SYMBOL(fscrypt_process_policy);
  67 +EXPORT_SYMBOL(fscrypt_ioctl_set_policy);
  68
  69 -int fscrypt_get_policy(struct inode *inode, struct fscrypt_policy *policy)
  70 +int fscrypt_ioctl_get_policy(struct file *filp, void __user *arg)
  71  {
  72 +       struct inode *inode = file_inode(filp);
  73         struct fscrypt_context ctx;
  74 +       struct fscrypt_policy policy;
  75         int res;
  76
  77         if (!inode->i_sb->s_cop->get_context ||
  78 @@ -151,15 +156,18 @@ int fscrypt_get_policy(struct inode *inode, struct fscrypt_policy *policy)
  79         if (ctx.format != FS_ENCRYPTION_CONTEXT_FORMAT_V1)
  80                 return -EINVAL;
  81
  82 -       policy->version = 0;
  83 -       policy->contents_encryption_mode = ctx.contents_encryption_mode;
  84 -       policy->filenames_encryption_mode = ctx.filenames_encryption_mode;
  85 -       policy->flags = ctx.flags;
  86 -       memcpy(&policy->master_key_descriptor, ctx.master_key_descriptor,
  87 +       policy.version = 0;
  88 +       policy.contents_encryption_mode = ctx.contents_encryption_mode;
  89 +       policy.filenames_encryption_mode = ctx.filenames_encryption_mode;
  90 +       policy.flags = ctx.flags;
  91 +       memcpy(policy.master_key_descriptor, ctx.master_key_descriptor,
  92                                 FS_KEY_DESCRIPTOR_SIZE);
  93 +
  94 +       if (copy_to_user(arg, &policy, sizeof(policy)))
  95 +               return -EFAULT;
  96         return 0;
  97  }
  98 -EXPORT_SYMBOL(fscrypt_get_policy);
  99 +EXPORT_SYMBOL(fscrypt_ioctl_get_policy);
 100
 101  int fscrypt_has_permitted_context(struct inode *parent, struct inode *child)
 102  {
 103 diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
 104 index aff204f040fc..f36bf98ef897 100644
 105 --- a/fs/ext4/ext4.h
 106 +++ b/fs/ext4/ext4.h
 107 @@ -2333,8 +2333,8 @@ static inline void ext4_fname_free_filename(struct ext4_filename *fname) { }
 108  #define fscrypt_pullback_bio_page      fscrypt_notsupp_pullback_bio_page
 109  #define fscrypt_restore_control_page   fscrypt_notsupp_restore_control_page
 110  #define fscrypt_zeroout_range          fscrypt_notsupp_zeroout_range
 111 -#define fscrypt_process_policy         fscrypt_notsupp_process_policy
 112 -#define fscrypt_get_policy             fscrypt_notsupp_get_policy
 113 +#define fscrypt_ioctl_set_policy       fscrypt_notsupp_ioctl_set_policy
 114 +#define fscrypt_ioctl_get_policy       fscrypt_notsupp_ioctl_get_policy
 115  #define fscrypt_has_permitted_context  fscrypt_notsupp_has_permitted_context
 116  #define fscrypt_inherit_context                fscrypt_notsupp_inherit_context
 117  #define fscrypt_get_encryption_info    fscrypt_notsupp_get_encryption_info
 118 diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
 119 index fc1cd37ba2d9..46c38d0f18ea 100644
 120 --- a/fs/ext4/ioctl.c
 121 +++ b/fs/ext4/ioctl.c
 122 @@ -768,22 +768,12 @@ long ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 123         }
 124         case EXT4_IOC_PRECACHE_EXTENTS:
 125                 return ext4_ext_precache(inode);
 126 -       case EXT4_IOC_SET_ENCRYPTION_POLICY: {
 127 -#ifdef CONFIG_EXT4_FS_ENCRYPTION
 128 -               struct fscrypt_policy policy;
 129
 130 +       case EXT4_IOC_SET_ENCRYPTION_POLICY:
 131                 if (!ext4_has_feature_encrypt(sb))
 132                         return -EOPNOTSUPP;
 133 +               return fscrypt_ioctl_set_policy(filp, (const void __user *)arg);
 134
 135 -               if (copy_from_user(&policy,
 136 -                                  (struct fscrypt_policy __user *)arg,
 137 -                                  sizeof(policy)))
 138 -                       return -EFAULT;
 139 -               return fscrypt_process_policy(filp, &policy);
 140 -#else
 141 -               return -EOPNOTSUPP;
 142 -#endif
 143 -       }
 144         case EXT4_IOC_GET_ENCRYPTION_PWSALT: {
 145                 int err, err2;
 146                 struct ext4_sb_info *sbi = EXT4_SB(sb);
 147 @@ -820,23 +810,9 @@ long ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 148                         return -EFAULT;
 149                 return 0;
 150         }
 151 -       case EXT4_IOC_GET_ENCRYPTION_POLICY: {
 152 -#ifdef CONFIG_EXT4_FS_ENCRYPTION
 153 -               struct fscrypt_policy policy;
 154 -               int err = 0;
 155 -
 156 -               if (!ext4_encrypted_inode(inode))
 157 -                       return -ENOENT;
 158 -               err = fscrypt_get_policy(inode, &policy);
 159 -               if (err)
 160 -                       return err;
 161 -               if (copy_to_user((void __user *)arg, &policy, sizeof(policy)))
 162 -                       return -EFAULT;
 163 -               return 0;
 164 -#else
 165 -               return -EOPNOTSUPP;
 166 -#endif
 167 -       }
 168 +       case EXT4_IOC_GET_ENCRYPTION_POLICY:
 169 +               return fscrypt_ioctl_get_policy(filp, (void __user *)arg);
 170 +
 171         case EXT4_IOC_FSGETXATTR:
 172         {
 173                 struct fsxattr fa;
 174 diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
 175 index 9e8de18a168a..8e94b7bda42b 100644
 176 --- a/fs/f2fs/f2fs.h
 177 +++ b/fs/f2fs/f2fs.h
 178 @@ -2453,8 +2453,8 @@ static inline bool f2fs_may_encrypt(struct inode *inode)
 179  #define fscrypt_pullback_bio_page      fscrypt_notsupp_pullback_bio_page
 180  #define fscrypt_restore_control_page   fscrypt_notsupp_restore_control_page
 181  #define fscrypt_zeroout_range          fscrypt_notsupp_zeroout_range
 182 -#define fscrypt_process_policy         fscrypt_notsupp_process_policy
 183 -#define fscrypt_get_policy             fscrypt_notsupp_get_policy
 184 +#define fscrypt_ioctl_set_policy       fscrypt_notsupp_ioctl_set_policy
 185 +#define fscrypt_ioctl_get_policy       fscrypt_notsupp_ioctl_get_policy
 186  #define fscrypt_has_permitted_context  fscrypt_notsupp_has_permitted_context
 187  #define fscrypt_inherit_context                fscrypt_notsupp_inherit_context
 188  #define fscrypt_get_encryption_info    fscrypt_notsupp_get_encryption_info
 189 diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
 190 index c7865073cd26..f0c83f74557d 100644
 191 --- a/fs/f2fs/file.c
 192 +++ b/fs/f2fs/file.c
 193 @@ -1752,31 +1752,16 @@ static bool uuid_is_nonzero(__u8 u[16])
 194
 195  static int f2fs_ioc_set_encryption_policy(struct file *filp, unsigned long arg)
 196  {
 197 -       struct fscrypt_policy policy;
 198         struct inode *inode = file_inode(filp);
 199
 200 -       if (copy_from_user(&policy, (struct fscrypt_policy __user *)arg,
 201 -                                                       sizeof(policy)))
 202 -               return -EFAULT;
 203 -
 204         f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);
 205
 206 -       return fscrypt_process_policy(filp, &policy);
 207 +       return fscrypt_ioctl_set_policy(filp, (const void __user *)arg);
 208  }
 209
 210  static int f2fs_ioc_get_encryption_policy(struct file *filp, unsigned long arg)
 211  {
 212 -       struct fscrypt_policy policy;
 213 -       struct inode *inode = file_inode(filp);
 214 -       int err;
 215 -
 216 -       err = fscrypt_get_policy(inode, &policy);
 217 -       if (err)
 218 -               return err;
 219 -
 220 -       if (copy_to_user((struct fscrypt_policy __user *)arg, &policy, sizeof(policy)))
 221 -               return -EFAULT;
 222 -       return 0;
 223 +       return fscrypt_ioctl_get_policy(filp, (void __user *)arg);
 224  }
 225
 226  static int f2fs_ioc_get_encryption_pwsalt(struct file *filp, unsigned long arg)
 227 diff --git a/include/linux/fscrypto.h b/include/linux/fscrypto.h
 228 index 98c71e973a96..be94684dc05f 100644
 229 --- a/include/linux/fscrypto.h
 230 +++ b/include/linux/fscrypto.h
 231 @@ -259,8 +259,8 @@ extern void fscrypt_restore_control_page(struct page *);
 232  extern int fscrypt_zeroout_range(const struct inode *, pgoff_t, sector_t,
 233                                                 unsigned int);
 234  /* policy.c */
 235 -extern int fscrypt_process_policy(struct file *, const struct fscrypt_policy *);
 236 -extern int fscrypt_get_policy(struct inode *, struct fscrypt_policy *);
 237 +extern int fscrypt_ioctl_set_policy(struct file *, const void __user *);
 238 +extern int fscrypt_ioctl_get_policy(struct file *, void __user *);
 239  extern int fscrypt_has_permitted_context(struct inode *, struct inode *);
 240  extern int fscrypt_inherit_context(struct inode *, struct inode *,
 241                                         void *, bool);
 242 @@ -334,14 +334,14 @@ static inline int fscrypt_notsupp_zeroout_range(const struct inode *i, pgoff_t p
 243  }
 244
 245  /* policy.c */
 246 -static inline int fscrypt_notsupp_process_policy(struct file *f,
 247 -                               const struct fscrypt_policy *p)
 248 +static inline int fscrypt_notsupp_ioctl_set_policy(struct file *f,
 249 +                               const void __user *arg)
 250  {
 251         return -EOPNOTSUPP;
 252  }
 253
 254 -static inline int fscrypt_notsupp_get_policy(struct inode *i,
 255 -                               struct fscrypt_policy *p)
 256 +static inline int fscrypt_notsupp_ioctl_get_policy(struct file *f,
 257 +                               void __user *arg)
 258  {
 259         return -EOPNOTSUPP;
 260  }