Re: [v10 4/5] ext4: adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR interface support

On Thu, Mar 19, 2015 at 5:56 PM, Jan Kara [off-list ref] wrote:

On Thu 19-03-15 04:04:56, Li Xi wrote:

quoted

This patch adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR ioctl interface
support for ext4. The interface is kept consistent with
XFS_IOC_FSGETXATTR/XFS_IOC_FSGETXATTR.

  Two more comments below.

quoted

Signed-off-by: Li Xi <redacted>

...

quoted

+static int ext4_ioctl_setproject(struct file *filp, __u32 projid)
+{
+     struct inode *inode = file_inode(filp);
+     struct super_block *sb = inode->i_sb;
+     struct ext4_inode_info *ei = EXT4_I(inode);
+     int err;
+     handle_t *handle;
+     kprojid_t kprojid;
+     struct ext4_iloc iloc;
+     struct ext4_inode *raw_inode;
+
+     struct dquot *transfer_to[EXT4_MAXQUOTAS] = { };
+
+     /* Make sure caller can change project. */
+     if (!capable(CAP_SYS_ADMIN))
+             return -EACCES;

  Why do you test capabilities here when you already test permission in
EXT4_IOC_FSSETXATTR? Furthermore this test is too restrictive. Just delete
it.

It seems we need this restrictive permission. Otherwise the owner of the file
can change the project ID to any other project. That means, the owner can
walk around the quota limit whenever he/she wants. But I agree checking
permission twice is not good.

...

quoted

-flags_out:
+             err = ext4_ioctl_setflags(inode, flags);
              mutex_unlock(&inode->i_mutex);
-             mnt_drop_write_file(filp);
+             mnt_drop_write(filp->f_path.mnt);

  Why did you change this? mnt_drop_write_file() should stay here.

Sorry, my mistake.

Regards,
Li Xi