From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
To: "J. Bruce Fields" <bfields@fieldses.org>
Cc: agruen@kernel.org, akpm@linux-foundation.org,
dhowells@redhat.com, linux-fsdevel@vger.kernel.org,
linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH -V6 09/26] vfs: Add delete child and delete self permission flags
Date: Fri, 09 Sep 2011 10:55:29 +0530 [thread overview]
Message-ID: <87obyu4792.fsf@skywalker.in.ibm.com> (raw)
In-Reply-To: <87r53q47jc.fsf@skywalker.in.ibm.com>
On Fri, 09 Sep 2011 10:49:19 +0530, "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
> On Thu, 8 Sep 2011 18:02:46 -0400, "J. Bruce Fields" <bfields@fieldses.org> wrote:
> > On Thu, Sep 08, 2011 at 04:07:54PM -0400, J. Bruce Fields wrote:
> > > On Thu, Sep 08, 2011 at 03:00:58PM +0530, Aneesh Kumar K.V wrote:
> > > > On Wed, 7 Sep 2011 16:39:16 -0400, "J. Bruce Fields" <bfields@fieldses.org> wrote:
> > > > > On Mon, Sep 05, 2011 at 10:55:31PM +0530, Aneesh Kumar K.V wrote:
> > > > > > +static int may_delete(struct inode *dir, struct dentry *victim,
> > > > > > + int isdir, int replace)
> > > > > > {
> > > > > > - int error;
> > > > > > + int mask, error, is_sticky;
> > > > > > + struct inode *inode = victim->d_inode;
> > > > > >
> > > > > > - if (!victim->d_inode)
> > > > > > + if (!inode)
> > > > > > return -ENOENT;
> > > > > >
> > > > > > BUG_ON(victim->d_parent->d_inode != dir);
> > > > > > audit_inode_child(victim, dir);
> > > > > >
> > > > > > - error = inode_permission(dir, MAY_WRITE | MAY_EXEC);
> > > > > > + mask = MAY_WRITE | MAY_EXEC | MAY_DELETE_CHILD;
> > > > > > + if (replace)
> > > > > > + mask |= S_ISDIR(inode->i_mode) ?
> > > > > > + MAY_CREATE_DIR : MAY_CREATE_FILE;
> > > > >
> > > > > I'm having trouble understanding this next bit:
> > > > >
> > > > > > + is_sticky = check_sticky(dir, inode);
> > > > > > + error = inode_permission(dir, mask);
> > > > > > + if ((error || is_sticky) && IS_RICHACL(inode) &&
> > > > > > + !inode_permission(dir, mask & ~(MAY_WRITE | MAY_DELETE_CHILD)) &&
> > > > > > + !inode_permission(inode, MAY_DELETE_SELF))
> > > > > > + error = 0;
> > > > >
> > > > > OK, so we can ignore the lack of write or delete permissions on the
> > > > > parent if we have delete_self permissions on the child. I guess that's
> > > > > right.
> > > > >
> > > > > Why the "|| is_sticky" above?
> > > > >
> > > > > Is there some less complicated why to write this?
> > > >
> > > > we removed the ns_capable check out of check_sticky, because we don't
> > > > want to do capability check when richacl allows access. We also want to
> > > > make sure that even if mode bits allow access (inode_permission(dir, mask))
> > > > if sticky bit is set we do additional check.
> > >
> > > Why are the two inode_permissions ANDed? The windows semantics are that
> > > you can delete if you have MAY_DELETE_CHILD *or* MAY_DELETE_SELF.
> >
> > Either way, those conditions are just really hard to follow. Could you
> > simplify the logic, add comments, maybe move the richacl stuff into a
> > little helper function?
> >
> > Also, a nit:
> >
> > > > > > + !inode_permission(dir, mask & ~(MAY_WRITE | MAY_DELETE_CHILD)) &&
> >
> > The way you calculated mask above it always includes MAY_WRITE and
> > MAY_DELETE_CHILD, so the above is equivalent to just
> >
> > !inode_permission(dir, MAY_WRITE | MAY_DELETE_CHILD) &&
> >
> > isn't it?
> >
>
> I guess i can simplify it as
> !inode_permission(dir, MAY_EXEC | replace_mask)
>
diff --git a/fs/namei.c b/fs/namei.c
index 1054bc3..e545c81 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1912,8 +1912,9 @@ other_userns:
static int may_delete(struct inode *dir, struct dentry *victim,
int isdir, int replace)
{
- int mask, error, is_sticky;
struct inode *inode = victim->d_inode;
+ int mask, replace_mask = 0, error, is_sticky;
+
if (!inode)
return -ENOENT;
@@ -1923,12 +1924,12 @@ static int may_delete(struct inode *dir, struct dentry *victim,
mask = MAY_WRITE | MAY_EXEC | MAY_DELETE_CHILD;
if (replace)
- mask |= S_ISDIR(inode->i_mode) ?
- MAY_CREATE_DIR : MAY_CREATE_FILE;
+ replace_mask = S_ISDIR(inode->i_mode) ?
+ MAY_CREATE_DIR : MAY_CREATE_FILE;
is_sticky = check_sticky(dir, inode);
- error = inode_permission(dir, mask);
+ error = inode_permission(dir, mask | replace_mask);
if ((error || is_sticky) && IS_RICHACL(inode) &&
- !inode_permission(dir, mask & ~(MAY_WRITE | MAY_DELETE_CHILD)) &&
+ !inode_permission(dir, MAY_EXEC | replace_mask) &&
!inode_permission(inode, MAY_DELETE_SELF))
error = 0;
else if (!error && is_sticky &&
next prev parent reply other threads:[~2011-09-09 5:26 UTC|newest]
Thread overview: 79+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-09-05 17:25 [PATCH -V6 00/26] New ACL format for better NFSv4 acl interoperability Aneesh Kumar K.V
2011-09-05 17:25 ` [PATCH -V6 01/26] vfs: Indicate that the permission functions take all the MAY_* flags Aneesh Kumar K.V
2011-09-05 17:25 ` [PATCH -V6 02/26] vfs: Add hex format for MAY_* flag values Aneesh Kumar K.V
2011-09-05 17:25 ` [PATCH -V6 03/26] vfs: Pass all mask flags down to iop->check_acl Aneesh Kumar K.V
2011-09-05 17:25 ` [PATCH -V6 04/26] vfs: Add a comment to inode_permission() Aneesh Kumar K.V
2011-09-05 17:25 ` [PATCH -V6 05/26] vfs: Add generic IS_ACL() test for acl support Aneesh Kumar K.V
2011-09-05 17:25 ` [PATCH -V6 06/26] vfs: Add IS_RICHACL() test for richacl support Aneesh Kumar K.V
2011-09-05 17:25 ` [PATCH -V6 07/26] vfs: Optimize out IS_RICHACL() if CONFIG_FS_RICHACL is not defined Aneesh Kumar K.V
2011-09-05 17:25 ` [PATCH -V6 08/26] vfs: Add new file and directory create permission flags Aneesh Kumar K.V
2011-09-05 17:25 ` [PATCH -V6 09/26] vfs: Add delete child and delete self " Aneesh Kumar K.V
2011-09-07 20:39 ` J. Bruce Fields
2011-09-08 9:30 ` Aneesh Kumar K.V
2011-09-08 20:07 ` J. Bruce Fields
2011-09-08 22:02 ` J. Bruce Fields
2011-09-09 5:19 ` Aneesh Kumar K.V
2011-09-09 5:25 ` Aneesh Kumar K.V [this message]
2011-09-09 12:02 ` J. Bruce Fields
2011-09-09 5:14 ` Aneesh Kumar K.V
2011-09-05 17:25 ` [PATCH -V6 10/26] vfs: Make the inode passed to inode_change_ok non-const Aneesh Kumar K.V
2011-09-07 20:43 ` J. Bruce Fields
2011-09-08 9:32 ` Aneesh Kumar K.V
2011-09-05 17:25 ` [PATCH -V6 11/26] vfs: Add permission flags for setting file attributes Aneesh Kumar K.V
2011-09-07 20:55 ` J. Bruce Fields
2011-09-08 9:36 ` Aneesh Kumar K.V
2011-09-08 20:08 ` J. Bruce Fields
2011-09-05 17:25 ` [PATCH -V6 12/26] vfs: Make acl_permission_check() work for richacls Aneesh Kumar K.V
2011-09-05 17:25 ` [PATCH -V6 13/26] richacl: In-memory representation and helper functions Aneesh Kumar K.V
2011-09-05 17:25 ` [PATCH -V6 14/26] richacl: Permission mapping functions Aneesh Kumar K.V
2011-09-07 21:24 ` J. Bruce Fields
2011-09-08 10:27 ` Aneesh Kumar K.V
2011-09-05 17:25 ` [PATCH -V6 15/26] richacl: Compute maximum file masks from an acl Aneesh Kumar K.V
2011-09-05 17:25 ` [PATCH -V6 16/26] richacl: Update the file masks in chmod() Aneesh Kumar K.V
2011-09-05 17:25 ` [PATCH -V6 17/26] richacl: Permission check algorithm Aneesh Kumar K.V
2011-09-07 21:50 ` J. Bruce Fields
2011-09-08 10:34 ` Aneesh Kumar K.V
2011-09-05 17:25 ` [PATCH -V6 18/26] richacl: Create-time inheritance Aneesh Kumar K.V
2011-09-05 17:25 ` [PATCH -V6 19/26] richacl: Check if an acl is equivalent to a file mode Aneesh Kumar K.V
2011-09-05 17:25 ` [PATCH -V6 20/26] richacl: Automatic Inheritance Aneesh Kumar K.V
2011-09-07 21:56 ` J. Bruce Fields
2011-09-05 17:25 ` [PATCH -V6 21/26] richacl: xattr mapping functions Aneesh Kumar K.V
2011-09-05 17:25 ` [PATCH -V6 22/26] vfs: Cache richacl in struct inode Aneesh Kumar K.V
2011-09-05 17:25 ` [PATCH -V6 23/26] vfs: Add richacl permission check Aneesh Kumar K.V
2011-09-05 17:25 ` [PATCH -V6 24/26] ext4: Use IS_POSIXACL() to check for POSIX ACL support Aneesh Kumar K.V
2011-09-05 17:25 ` [PATCH -V6 25/26] ext4: Implement rich acl for ext4 Aneesh Kumar K.V
2011-09-05 17:25 ` [PATCH -V6 26/26] ext4: Add temporary richacl mount option " Aneesh Kumar K.V
2011-09-05 22:42 ` [PATCH -V6 00/26] New ACL format for better NFSv4 acl interoperability Casey Schaufler
2011-09-08 0:46 ` Valdis.Kletnieks
2011-09-12 21:34 ` Casey Schaufler
2011-09-12 22:20 ` J. Bruce Fields
2011-09-12 22:38 ` Casey Schaufler
2011-09-12 22:43 ` J. Bruce Fields
2011-09-12 23:23 ` Casey Schaufler
2011-09-12 23:53 ` J. Bruce Fields
2011-09-13 4:41 ` Aneesh Kumar K.V
2011-09-13 18:12 ` Valdis.Kletnieks
2011-09-06 9:41 ` Steven Whitehouse
2011-09-06 13:58 ` Aneesh Kumar K.V
2011-09-07 20:18 ` J. Bruce Fields
2011-09-07 23:44 ` J. Bruce Fields
2011-09-08 10:40 ` Aneesh Kumar K.V
2011-09-09 10:02 ` [PATCH -V6 08/26] vfs: Add new file and directory create permission flags David Howells
2011-09-09 11:59 ` Aneesh Kumar K.V
2011-09-09 10:12 ` [PATCH -V6 09/26] vfs: Add delete child and delete self " David Howells
2011-09-09 11:55 ` Aneesh Kumar K.V
2011-09-09 10:36 ` [PATCH -V6 14/26] richacl: Permission mapping functions David Howells
2011-09-09 11:54 ` Aneesh Kumar K.V
2011-09-09 12:14 ` [PATCH -V6 18/26] richacl: Create-time inheritance David Howells
2011-09-09 12:37 ` [PATCH -V6 22/26] vfs: Cache richacl in struct inode David Howells
2011-09-09 12:45 ` [PATCH -V6 25/26] ext4: Implement rich acl for ext4 David Howells
2011-09-13 4:25 ` Aneesh Kumar K.V
2011-09-09 12:48 ` [PATCH -V6 00/26] New ACL format for better NFSv4 acl interoperability David Howells
2011-09-09 14:03 ` David Howells
2011-09-12 22:23 ` J. Bruce Fields
2011-09-12 22:27 ` David Howells
2011-09-13 5:07 ` Aneesh Kumar K.V
2011-09-21 7:30 ` Aneesh Kumar K.V
2011-09-21 19:45 ` J. Bruce Fields
2011-09-22 5:31 ` Aneesh Kumar K.V
2011-09-22 12:01 ` J. Bruce Fields
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87obyu4792.fsf@skywalker.in.ibm.com \
--to=aneesh.kumar@linux.vnet.ibm.com \
--cc=agruen@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=bfields@fieldses.org \
--cc=dhowells@redhat.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).