| 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
| ||
|
Date: Thu, 08 Sep 2011 15:57:37 +0530
From: "Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
To: "J. Bruce Fields" <bfields@...ldses.org>, agruen@...bit.com
Cc: agruen@...nel.org, akpm@...ux-foundation.org, dhowells@...hat.com,
linux-fsdevel@...r.kernel.org, linux-nfs@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH -V6 14/26] richacl: Permission mapping functions
On Wed, 7 Sep 2011 17:24:00 -0400, "J. Bruce Fields" <bfields@...ldses.org> wrote:
> On Mon, Sep 05, 2011 at 10:55:36PM +0530, Aneesh Kumar K.V wrote:
> > From: Andreas Gruenbacher <agruen@...nel.org>
> >
> > We need to map from POSIX permissions to NFSv4 permissions when a
> > chmod() is done, from NFSv4 permissions to POSIX permissions when an acl
> > is set (which implicitly sets the file permission bits), and from the
> > MAY_READ/MAY_WRITE/MAY_EXEC/MAY_APPEND flags to NFSv4 permissions when
> > doing an access check in a richacl.
> >
> > Signed-off-by: Andreas Gruenbacher <agruen@...nel.org>
> > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@...ux.vnet.ibm.com>
> > ---
> > fs/richacl_base.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++
> > include/linux/richacl.h | 46 ++++++++++++++++++
> > 2 files changed, 164 insertions(+), 0 deletions(-)
> >
> > diff --git a/fs/richacl_base.c b/fs/richacl_base.c
> > index 3536626..d55b436 100644
> > --- a/fs/richacl_base.c
> > +++ b/fs/richacl_base.c
> > @@ -69,6 +69,124 @@ richacl_clone(const struct richacl *acl)
> > }
> >
> > /**
> > + * richacl_mask_to_mode - compute the file permission bits which correspond to @mask
> > + * @mask: %ACE4_* permission mask
> > + *
> > + * See richacl_masks_to_mode().
> > + */
> > +static int
> > +richacl_mask_to_mode(unsigned int mask)
> > +{
> > + int mode = 0;
> > +
> > + if (mask & ACE4_POSIX_MODE_READ)
> > + mode |= MAY_READ;
> > + if (mask & ACE4_POSIX_MODE_WRITE)
> > + mode |= MAY_WRITE;
> > + if (mask & ACE4_POSIX_MODE_EXEC)
> > + mode |= MAY_EXEC;
> > +
> > + return mode;
> > +}
> > +
> > +/**
> > + * richacl_masks_to_mode - compute the file permission bits from the file masks
> > + *
> > + * When setting a richacl, we set the file permission bits to indicate maximum
> > + * permissions: for example, we set the Write permission when a mask contains
> > + * ACE4_APPEND_DATA even if it does not also contain ACE4_WRITE_DATA.
> > + *
> > + * Permissions which are not in ACE4_POSIX_MODE_READ, ACE4_POSIX_MODE_WRITE, or
> > + * ACE4_POSIX_MODE_EXEC cannot be represented in the file permission bits.
> > + * Such permissions can still be effective, but not for new files or after a
> > + * chmod(), and only if they were set explicitly, for example, by setting a
> > + * richacl.
> > + */
> > +int
> > +richacl_masks_to_mode(const struct richacl *acl)
> > +{
> > + return richacl_mask_to_mode(acl->a_owner_mask) << 6 |
> > + richacl_mask_to_mode(acl->a_group_mask) << 3 |
> > + richacl_mask_to_mode(acl->a_other_mask);
> > +}
> > +EXPORT_SYMBOL_GPL(richacl_masks_to_mode);
> > +
> > +/**
> > + * richacl_mode_to_mask - compute a file mask from the lowest three mode bits
> > + *
> > + * When the file permission bits of a file are set with chmod(), this specifies
> > + * the maximum permissions that processes will get. All permissions beyond
> > + * that will be removed from the file masks, and become ineffective.
> > + *
> > + * We also add in the permissions which are always allowed no matter what the
> > + * acl says.
> > + */
> > +unsigned int
> > +richacl_mode_to_mask(mode_t mode)
> > +{
> > + unsigned int mask = ACE4_POSIX_ALWAYS_ALLOWED;
> > +
> > + if (mode & MAY_READ)
> > + mask |= ACE4_POSIX_MODE_READ;
> > + if (mode & MAY_WRITE)
> > + mask |= ACE4_POSIX_MODE_WRITE;
> > + if (mode & MAY_EXEC)
> > + mask |= ACE4_POSIX_MODE_EXEC;
> > +
> > + return mask;
> > +}
> > +
> > +/**
> > + * richacl_want_to_mask - convert the iop->permission want argument to a mask
> > + * @want: @want argument of the permission inode operation
> > + *
> > + * When checking for append, @want is (MAY_WRITE | MAY_APPEND).
> > + *
> > + * Richacls use the iop->may_create and iop->may_delete hooks which are
> > + * used for checking if creating and deleting files is allowed. These hooks do
> > + * not use richacl_want_to_mask(), so we do not have to deal with mapping
> > + * MAY_WRITE to ACE4_ADD_FILE, ACE4_ADD_SUBDIRECTORY, and ACE4_DELETE_CHILD
> > + * here.
> > + */
> > +unsigned int
> > +richacl_want_to_mask(int want)
> > +{
> > + unsigned int mask = 0;
> > +
> > + if (want & MAY_READ)
> > + mask |= ACE4_READ_DATA;
> > + if (want & (MAY_APPEND |
> > + MAY_CREATE_FILE | MAY_CREATE_DIR |
> > + MAY_DELETE_CHILD | MAY_DELETE_SELF |
> > + MAY_TAKE_OWNERSHIP | MAY_CHMOD | MAY_SET_TIMES)) {
> > + if (want & MAY_APPEND)
> > + mask |= ACE4_APPEND_DATA;
> > + else if (want & MAY_DELETE_SELF)
> > + mask |= ACE4_DELETE;
> > + else if (want & MAY_TAKE_OWNERSHIP)
> > + mask |= ACE4_WRITE_OWNER;
> > + else if (want & MAY_CHMOD)
> > + mask |= ACE4_WRITE_ACL;
> > + else if (want & MAY_SET_TIMES)
> > + mask |= ACE4_WRITE_ATTRIBUTES;
> > + else {
> > + if (want & MAY_CREATE_FILE)
> > + mask |= ACE4_ADD_FILE;
> > + if (want & MAY_CREATE_DIR)
> > + mask |= ACE4_ADD_SUBDIRECTORY;
> > + if (want & MAY_DELETE_CHILD)
> > + mask |= ACE4_DELETE_CHILD;
> > + }
>
> Possibly dumb question: why isn't this whole function a simple series of
> if's, one for each MAY_ bit?
>
> I guess you're using knowledge about the callers to know that, for
> example, no one will ask for MAY_APPEND and MAY_TAKE_OWNERSHIP at the
> same time?
>
> And adding that big "if (want & (MAY_APPEND | .. | MAY_SET_TIMES))" to
> let you skip over a bunch of checks in the common case?
>
> Does this help measurably? It seems complicated and, to the extent it
> makes assumptions about the callers, possibly fragile with respect to
> future changes.
>
Not the complete function. But I guess we can do the below change.
We still want to keep MAY_WRITE check separate, because VFS do add
MAY_WRITE request to different type of request other than write.
Andreas,
Do you see any issue in doing below ?
richacl_want_to_mask(int want)
{
unsigned int mask = 0;
if (want & MAY_READ)
mask |= ACE4_READ_DATA;
if (want & MAY_DELETE_SELF)
mask |= ACE4_DELETE;
if (want & MAY_TAKE_OWNERSHIP)
mask |= ACE4_WRITE_OWNER;
if (want & MAY_CHMOD)
mask |= ACE4_WRITE_ACL;
if (want & MAY_SET_TIMES)
mask |= ACE4_WRITE_ATTRIBUTES;
if (want & MAY_EXEC)
mask |= ACE4_EXECUTE;
/*
* differentiate MAY_WRITE from these request
*/
if (want & (MAY_APPEND |
MAY_CREATE_FILE | MAY_CREATE_DIR |
MAY_DELETE_CHILD)) {
if (want & MAY_APPEND)
mask |= ACE4_APPEND_DATA;
if (want & MAY_CREATE_FILE)
mask |= ACE4_ADD_FILE;
if (want & MAY_CREATE_DIR)
mask |= ACE4_ADD_SUBDIRECTORY;
if (want & MAY_DELETE_CHILD)
mask |= ACE4_DELETE_CHILD;
} else if (want & MAY_WRITE)
mask |= ACE4_WRITE_DATA;
return mask;
}
-aneesh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists