Return-Path: Received: from mail-qk0-f178.google.com ([209.85.220.178]:36385 "EHLO mail-qk0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755033AbcGELSp (ORCPT ); Tue, 5 Jul 2016 07:18:45 -0400 Received: by mail-qk0-f178.google.com with SMTP id u68so8690420qkc.3 for ; Tue, 05 Jul 2016 04:18:44 -0700 (PDT) Message-ID: <1467717521.3800.11.camel@redhat.com> Subject: Re: [PATCH v23 05/22] vfs: Add permission flags for setting file attributes From: Jeff Layton To: Andreas Gruenbacher , Alexander Viro Cc: Christoph Hellwig , "Theodore Ts'o" , Andreas Dilger , "J. Bruce Fields" , Trond Myklebust , Anna Schumaker , Dave Chinner , linux-ext4@vger.kernel.org, xfs@oss.sgi.com, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-nfs@vger.kernel.org, linux-cifs@vger.kernel.org, linux-api@vger.kernel.org Date: Tue, 05 Jul 2016 07:18:41 -0400 In-Reply-To: <1467294433-3222-6-git-send-email-agruenba@redhat.com> References: <1467294433-3222-1-git-send-email-agruenba@redhat.com> <1467294433-3222-6-git-send-email-agruenba@redhat.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Sender: linux-nfs-owner@vger.kernel.org List-ID: On Thu, 2016-06-30 at 15:46 +0200, Andreas Gruenbacher wrote: > Richacls support permissions that allow to take ownership of a file, > change the file permissions, and set the file timestamps.  Support that > by introducing new permission mask flags and by checking for those mask > flags in inode_change_ok(). > > Signed-off-by: Andreas Gruenbacher > Reviewed-by: J. Bruce Fields > Reviewed-by: Steve French > --- >  fs/attr.c          | 79 +++++++++++++++++++++++++++++++++++++++++++++--------- >  include/linux/fs.h |  3 +++ >  2 files changed, 70 insertions(+), 12 deletions(-) > > diff --git a/fs/attr.c b/fs/attr.c > index 7ca7fa0..2a8c49c 100644 > --- a/fs/attr.c > +++ b/fs/attr.c > @@ -17,6 +17,65 @@ >  #include >   >  /** > + * inode_extended_permission  -  permissions beyond read/write/execute > + * > + * Check for permissions that only richacls can currently grant. > + */ > +static int inode_extended_permission(struct inode *inode, int mask) > +{ > + if (!IS_RICHACL(inode)) > + return -EPERM; > + return inode_permission(inode, mask); > +} > + > +static bool inode_uid_change_ok(struct inode *inode, kuid_t ia_uid) > +{ > + if (uid_eq(current_fsuid(), inode->i_uid) && > +     uid_eq(ia_uid, inode->i_uid)) > + return true; > + if (uid_eq(current_fsuid(), ia_uid) && > +     inode_extended_permission(inode, MAY_TAKE_OWNERSHIP) == 0) > + return true; > + if (capable_wrt_inode_uidgid(inode, CAP_CHOWN)) > + return true; > + return false; > +} > + > +static bool inode_gid_change_ok(struct inode *inode, kgid_t ia_gid) > +{ > + int in_group = in_group_p(ia_gid); > + if (uid_eq(current_fsuid(), inode->i_uid) && > +     (in_group || gid_eq(ia_gid, inode->i_gid))) > + return true; > + if (in_group && inode_extended_permission(inode, MAY_TAKE_OWNERSHIP) == 0) > + return true; > + if (capable_wrt_inode_uidgid(inode, CAP_CHOWN)) > + return true; > + return false; > +} > + > +/** > + * inode_owner_permitted_or_capable > + * > + * Check for permissions implicitly granted to the owner, like MAY_CHMOD or > + * MAY_SET_TIMES.  Equivalent to inode_owner_or_capable for file systems > + * without support for those permissions. > + */ > +static bool inode_owner_permitted_or_capable(struct inode *inode, int mask) > +{ > + struct user_namespace *ns; > + > + if (uid_eq(current_fsuid(), inode->i_uid)) > + return true; > + if (inode_extended_permission(inode, mask) == 0) > + return true; > + ns = current_user_ns(); > + if (ns_capable(ns, CAP_FOWNER) && kuid_has_mapping(ns, inode->i_uid)) > + return true; > + return false; > +} > + > +/** >   * inode_change_ok - check if attribute changes to an inode are allowed >   * @inode: inode to check >   * @attr: attributes to change > @@ -47,22 +106,18 @@ int inode_change_ok(struct inode *inode, struct iattr *attr) >   return 0; >   >   /* Make sure a caller can chown. */ > - if ((ia_valid & ATTR_UID) && > -     (!uid_eq(current_fsuid(), inode->i_uid) || > -      !uid_eq(attr->ia_uid, inode->i_uid)) && > -     !capable_wrt_inode_uidgid(inode, CAP_CHOWN)) > - return -EPERM; > + if (ia_valid & ATTR_UID) > + if (!inode_uid_change_ok(inode, attr->ia_uid)) > + return -EPERM; >   >   /* Make sure caller can chgrp. */ > - if ((ia_valid & ATTR_GID) && > -     (!uid_eq(current_fsuid(), inode->i_uid) || > -     (!in_group_p(attr->ia_gid) && !gid_eq(attr->ia_gid, inode->i_gid))) && > -     !capable_wrt_inode_uidgid(inode, CAP_CHOWN)) > - return -EPERM; > + if (ia_valid & ATTR_GID) > + if (!inode_gid_change_ok(inode, attr->ia_gid)) > + return -EPERM; >   >   /* Make sure a caller can chmod. */ >   if (ia_valid & ATTR_MODE) { > - if (!inode_owner_or_capable(inode)) > + if (!inode_owner_permitted_or_capable(inode, MAY_CHMOD)) >   return -EPERM; >   /* Also check the setgid bit! */ >   if (!in_group_p((ia_valid & ATTR_GID) ? attr->ia_gid : > @@ -73,7 +128,7 @@ int inode_change_ok(struct inode *inode, struct iattr *attr) >   >   /* Check for setting the inode time. */ >   if (ia_valid & (ATTR_MTIME_SET | ATTR_ATIME_SET | ATTR_TIMES_SET)) { > - if (!inode_owner_or_capable(inode)) > + if (!inode_owner_permitted_or_capable(inode, MAY_SET_TIMES)) >   return -EPERM; >   } >   > diff --git a/include/linux/fs.h b/include/linux/fs.h > index 208f73b..bb36561 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -88,6 +88,9 @@ typedef int (dio_iodone_t)(struct kiocb *iocb, loff_t offset, >  #define MAY_CREATE_DIR 0x00000200 >  #define MAY_DELETE_CHILD 0x00000400 >  #define MAY_DELETE_SELF 0x00000800 > +#define MAY_TAKE_OWNERSHIP 0x00001000 > +#define MAY_CHMOD 0x00002000 > +#define MAY_SET_TIMES 0x00004000 >   >  /* >   * flags in file.f_mode.  Note that FMODE_READ and FMODE_WRITE must correspond Reviewed-by: Jeff Layton