linux-ext4 - Re: [v10 4/5] ext4: adds FS_IOC_FSSETXATTR/FS_IOC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPTn0cAkHZ7xhrR2RPpDqffX-Mndt7aPQ-PTHb3tkfCdUWLPBA@mail.gmail.com>
Date:	Fri, 20 Mar 2015 21:24:07 +0800
From:	Li Xi <pkuelelixi@...il.com>
To:	Jan Kara <jack@...e.cz>
Cc:	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
	Ext4 Developers List <linux-ext4@...r.kernel.org>,
	"linux-api@...r.kernel.org" <linux-api@...r.kernel.org>,
	"Theodore Ts'o" <tytso@....edu>,
	Andreas Dilger <adilger@...ger.ca>,
	"viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>,
	"hch@...radead.org" <hch@...radead.org>,
	Dmitry Monakhov <dmonakhov@...nvz.org>
Subject: 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 <jack@...e.cz> 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 <lixi@....com>
> ...
>> +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-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html