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
