On Thu, Mar 19, 2015 at 5:56 PM, Jan Kara <[email protected]> wrote: > On Thu 19-03-15 04:04:56, Li Xi wrote: >> 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. > >> >> Signed-off-by: Li Xi <[email protected]> > ... >> +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. > ... >> -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 -- To unsubscribe from this list: send the line "unsubscribe linux-api" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
