Re: [PATCH v27 03/21] vfs: Add MAY_DELETE_SELF and MAY_DELETE_CHILD permission flags

From: Andreas Gruenbacher
Date: Mon Feb 13 2017 - 10:42:42 EST


On Fri, Dec 2, 2016 at 10:57 AM, Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
> On Tue, Oct 11, 2016 at 2:50 PM, Andreas Gruenbacher
> <agruenba@xxxxxxxxxx> wrote:
>> Normally, deleting a file requires MAY_WRITE access to the parent
>> directory. With richacls, a file may be deleted with MAY_DELETE_CHILD access
>> to the parent directory or with MAY_DELETE_SELF access to the file.
>>
>> To support that, pass the MAY_DELETE_CHILD mask flag to inode_permission()
>> when checking for delete access inside a directory, and MAY_DELETE_SELF
>> when checking for delete access to a file itself.
>>
>> The MAY_DELETE_SELF permission overrides the sticky directory check.
>
> And MAY_DELETE_SELF seems totally inappropriate to any kind of rename,
> since from the point of view of the inode we are not doing anything at
> all. The modifications are all in the parent(s), and that's where the
> permission checks need to be.
>
>> @@ -2780,14 +2780,20 @@ static int may_delete_or_replace(struct inode *dir, struct dentry *victim,
>> BUG_ON(victim->d_parent->d_inode != dir);
>> audit_inode_child(dir, victim, AUDIT_TYPE_CHILD_DELETE);
>>
>> - error = inode_permission(dir, mask);
>> + error = inode_permission(dir, mask | MAY_WRITE | MAY_DELETE_CHILD);
>> + if (!error && check_sticky(dir, inode))
>> + error = -EPERM;
>> + if (error && IS_RICHACL(inode) &&
>> + inode_permission(inode, MAY_DELETE_SELF) == 0 &&
>> + inode_permission(dir, mask) == 0)
>> + error = 0;
>
> Why is MAY_WRITE missing here? Everything not aware of
> MAY_DELETE_SELF (e.g. LSMs) will still need MAY_WRITE otherwise this
> is going to be a loophole.

Hmm, this has indeed slipped me. Should be fixed in the version I've
just posted.

Many thanks for the review.

Andreas