fix-setattr-project-check-in-fssetxattr-ioctl

   1 ext4: fix setattr project check in fssetxattr ioctl
   2
   3 From: Wang Shilong <wangshilong1991@gmail.com>
   4
   5 Currently, project quota could be changed by fssetxattr
   6 ioctl, and existed permission check inode_owner_or_capable()
   7 is obviously not enough, just think that common users could
   8 change project id of file, that could make users to
   9 break project quota easily.
  10
  11 This patch try to follow same regular of xfs project
  12 quota:
  13
  14 "Project Quota ID state is only allowed to change from
  15 within the init namespace. Enforce that restriction only
  16 if we are trying to change the quota ID state.
  17 Everything else is allowed in user namespaces."
  18
  19 Besides that, check and set project id'state should
  20 be an atomic operation, protect whole operation with
  21 inode lock, ext4_ioctl_setproject() is only used for
  22 ioctl EXT4_IOC_FSSETXATTR, we have held mnt_want_write_file()
  23 before ext4_ioctl_setflags(), and ext4_ioctl_setproject()
  24 is called after ext4_ioctl_setflags(), we could share
  25 codes, so remove it inside ext4_ioctl_setproject().
  26
  27 Signed-off-by: Wang Shilong <wshilong@ddn.com>
  28 Signed-off-by: Theodore Ts'o <tytso@mit.edu>
  29 Reviewed-by: Andreas Dilger <adilger@dilger.ca>
  30 Cc: stable@kernel.org
  31 ---
  32 v3->v4: explain why patch removed mnt_want/drop_write_file()
  33 inside ext4_ioctl_setproject() since diff file is
  34 not obvious for this.
  35 v2->v3: change function declaration to one line.
  36 v1->v2: rename ext4_ioctl_setattr_check_projid()
  37 to ext4_ioctl_check_project()
  38  fs/ext4/ioctl.c | 60 +++++++++++++++++++++++++++++++++++----------------------
  39  1 file changed, 37 insertions(+), 23 deletions(-)
  40
  41 diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
  42 index a707411..f81102b 100644
  43 --- a/fs/ext4/ioctl.c
  44 +++ b/fs/ext4/ioctl.c
  45 @@ -339,19 +339,14 @@ static int ext4_ioctl_setproject(struct file *filp, __u32 projid)
  46         if (projid_eq(kprojid, EXT4_I(inode)->i_projid))
  47                 return 0;
  48
  49 -       err = mnt_want_write_file(filp);
  50 -       if (err)
  51 -               return err;
  52 -
  53         err = -EPERM;
  54 -       inode_lock(inode);
  55         /* Is it quota file? Do not allow user to mess with it */
  56         if (ext4_is_quota_file(inode))
  57 -               goto out_unlock;
  58 +               return err;
  59
  60         err = ext4_get_inode_loc(inode, &iloc);
  61         if (err)
  62 -               goto out_unlock;
  63 +               return err;
  64
  65         raw_inode = ext4_raw_inode(&iloc);
  66         if (!EXT4_FITS_IN_INODE(raw_inode, ei, i_projid)) {
  67 @@ -359,7 +354,7 @@ static int ext4_ioctl_setproject(struct file *filp, __u32 projid)
  68                                               EXT4_SB(sb)->s_want_extra_isize,
  69                                               &iloc);
  70                 if (err)
  71 -                       goto out_unlock;
  72 +                       return err;
  73         } else {
  74                 brelse(iloc.bh);
  75         }
  76 @@ -369,10 +364,8 @@ static int ext4_ioctl_setproject(struct file *filp, __u32 projid)
  77         handle = ext4_journal_start(inode, EXT4_HT_QUOTA,
  78                 EXT4_QUOTA_INIT_BLOCKS(sb) +
  79                 EXT4_QUOTA_DEL_BLOCKS(sb) + 3);
  80 -       if (IS_ERR(handle)) {
  81 -               err = PTR_ERR(handle);
  82 -               goto out_unlock;
  83 -       }
  84 +       if (IS_ERR(handle))
  85 +               return PTR_ERR(handle);
  86
  87         err = ext4_reserve_inode_write(handle, inode, &iloc);
  88         if (err)
  89 @@ -400,9 +393,6 @@ static int ext4_ioctl_setproject(struct file *filp, __u32 projid)
  90                 err = rc;
  91  out_stop:
  92         ext4_journal_stop(handle);
  93 -out_unlock:
  94 -       inode_unlock(inode);
  95 -       mnt_drop_write_file(filp);
  96         return err;
  97  }
  98  #else
  99 @@ -626,6 +616,30 @@ static long ext4_ioctl_group_add(struct file *file,
 100         return err;
 101  }
 102
 103 +static int ext4_ioctl_check_project(struct inode *inode, struct fsxattr *fa)
 104 +{
 105 +       /*
 106 +        * Project Quota ID state is only allowed to change from within the init
 107 +        * namespace. Enforce that restriction only if we are trying to change
 108 +        * the quota ID state. Everything else is allowed in user namespaces.
 109 +        */
 110 +       if (current_user_ns() == &init_user_ns)
 111 +               return 0;
 112 +
 113 +       if (__kprojid_val(EXT4_I(inode)->i_projid) != fa->fsx_projid)
 114 +               return -EINVAL;
 115 +
 116 +       if (ext4_test_inode_flag(inode, EXT4_INODE_PROJINHERIT)) {
 117 +               if (!(fa->fsx_xflags & FS_XFLAG_PROJINHERIT))
 118 +                       return -EINVAL;
 119 +       } else {
 120 +               if (fa->fsx_xflags & FS_XFLAG_PROJINHERIT)
 121 +                       return -EINVAL;
 122 +       }
 123 +
 124 +       return 0;
 125 +}
 126 +
 127  long ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 128  {
 129         struct inode *inode = file_inode(filp);
 130 @@ -1025,19 +1039,19 @@ long ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 131                         return err;
 132
 133                 inode_lock(inode);
 134 +               err = ext4_ioctl_check_project(inode, &fa);
 135 +               if (err)
 136 +                       goto out;
 137                 flags = (ei->i_flags & ~EXT4_FL_XFLAG_VISIBLE) |
 138                          (flags & EXT4_FL_XFLAG_VISIBLE);
 139                 err = ext4_ioctl_setflags(inode, flags);
 140 -               inode_unlock(inode);
 141 -               mnt_drop_write_file(filp);
 142                 if (err)
 143 -                       return err;
 144 -
 145 +                       goto out;
 146                 err = ext4_ioctl_setproject(filp, fa.fsx_projid);
 147 -               if (err)
 148 -                       return err;
 149 -
 150 -               return 0;
 151 +out:
 152 +               inode_unlock(inode);
 153 +               mnt_drop_write_file(filp);
 154 +               return err;
 155         }
 156         case EXT4_IOC_SHUTDOWN:
 157                 return ext4_shutdown(sb, arg);
 158 --
 159 1.8.3.1
 160
 161