lkml.org 
[lkml]   [2011]   [Oct]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    SubjectRe: [PATCH -V7 21/26] richacl: xattr mapping functions
    Date
    On Fri, 21 Oct 2011 01:46:29 +0200, Andreas Gruenbacher <agruen@kernel.org> wrote:
    > On Thu, 2011-10-20 at 06:25 -0400, J. Bruce Fields wrote:
    > > On Thu, Oct 20, 2011 at 05:19:46AM -0400, Christoph Hellwig wrote:
    > > > On Thu, Oct 20, 2011 at 05:14:34AM -0400, J. Bruce Fields wrote:
    > > > > > > Does it really make sense to use a string here just to pick between the
    > > > > > > three choices OWNER@, GROUP@, and EVERYONE@? Why not just another small
    > > > > > > integer? Is the goal to expand this somehow eventually?
    > > > > >
    > > >
    > > > > > I guess Andreas wanted the disk layout to be able to store user@domain
    > > > > > format if needed.
    >
    > Yep. On the other hand, none of the code won't actually allow to use
    > user@domain identifiers, it won't help with other identifier types like
    > Windows SIDs, and it doesn't make the code any prettier, so this should
    > probably go away.
    >
    > > > > Is that likely? For that to be useful, tasks would need to be able to
    > > > > run as user@domain strings. And we'd probably want owners and groups to
    > > > > also be user@domain strings.
    >
    > I really don't see this happen anytime soon, and likely not at all.
    >
    > > > > The container people seem to eventually want to add some kind of
    > > > > namespace identifier everywhere:
    > > > >
    > > > > http://marc.info/?l=linux-kernel&m=131836778427871&w=2
    > > > >
    > > > > in which case I guess we'd likely end up with (uid, user namespace id)
    > > > > instead of user@domain?
    >
    > The filesystem still wouldn't have namespace ids for the owner and
    > owning group, which is a much bigger issue. I think we're safe not to
    > worry about namespace ids at this point; they also might never happen.
    >
    > > > Storing strings is an extremly stupid idea. The only thing that would
    > > > make sense would be storing a windows-style 128-bit GUID.
    > > >
    > >
    > > So if we want to do this without strings:
    > >
    > > > > > +struct richace_xattr {
    > > > > > + __le16 e_type;
    > > > > > + __le16 e_flags;
    > > > > > + __le32 e_mask;
    > > > > > + __le32 e_id;
    > > > > > + char e_who[0];
    > >
    > > We could drop that last field and use some predefined values for e_id to
    > > represent owner/group/everyone in the e_type == ACE4_SPECIAL_WHO case.
    >
    > That makes sense to me.
    >
    > There seems to be a WELL_KNOWN_SID_TYPE enumeration which maps those
    > kinds of special identifiers to small integers in Windows; maybe it
    > makes sense to use the same numbers for OWNER@, GROUP@, and EVERYONE@.
    >
    > > Then I'm not sure how you'd extend it if you later decided to add
    > > Windows GUID's or whatever.
    > >
    > > But maybe it's not realistic to expect to be able to do that without a
    > > new interface and on-disk format: how could old software be expected to
    > > deal with acls that didn't use uid's?
    >
    > The acl itself has a version field, so new formats could be introduced
    > in the future with a new version.

    How about the below change. This will require richacl tools change
    also. I made the e_flags 32 bit to make sure we don't take the space
    needed NFSv4 ACL related flags.

    diff --git a/fs/richacl_base.c b/fs/richacl_base.c
    index 9a57039..9179fcd 100644
    --- a/fs/richacl_base.c
    +++ b/fs/richacl_base.c
    @@ -20,19 +20,6 @@

    MODULE_LICENSE("GPL");

    -/*
    - * Special e_who identifiers: ACEs which have ACE4_SPECIAL_WHO set in
    - * ace->e_flags use these constants in ace->u.e_who.
    - *
    - * For efficiency, we compare pointers instead of comparing strings.
    - */
    -const char richace_owner_who[] = "OWNER@";
    -EXPORT_SYMBOL_GPL(richace_owner_who);
    -const char richace_group_who[] = "GROUP@";
    -EXPORT_SYMBOL_GPL(richace_group_who);
    -const char richace_everyone_who[] = "EVERYONE@";
    -EXPORT_SYMBOL_GPL(richace_everyone_who);
    -
    /**
    * richacl_alloc - allocate a richacl
    * @count: number of entries
    @@ -191,41 +178,14 @@ EXPORT_SYMBOL_GPL(richacl_want_to_mask);
    int
    richace_is_same_identifier(const struct richace *a, const struct richace *b)
    {
    -#define WHO_FLAGS (ACE4_SPECIAL_WHO | ACE4_IDENTIFIER_GROUP)
    +#define WHO_FLAGS (ACE4_SPECIAL_WHO | ACE4_UNIXID_WHO | ACE4_IDENTIFIER_GROUP)
    if ((a->e_flags & WHO_FLAGS) != (b->e_flags & WHO_FLAGS))
    return 0;
    - if (a->e_flags & ACE4_SPECIAL_WHO)
    - return a->u.e_who == b->u.e_who;
    - else
    - return a->u.e_id == b->u.e_id;
    + return a->e_id == b->e_id;
    #undef WHO_FLAGS
    }

    /**
    - * richacl_set_who - set a special who value
    - * @ace: acl entry
    - * @who: who value to use
    - */
    -int
    -richace_set_who(struct richace *ace, const char *who)
    -{
    - if (!strcmp(who, richace_owner_who))
    - who = richace_owner_who;
    - else if (!strcmp(who, richace_group_who))
    - who = richace_group_who;
    - else if (!strcmp(who, richace_everyone_who))
    - who = richace_everyone_who;
    - else
    - return -EINVAL;
    -
    - ace->u.e_who = who;
    - ace->e_flags |= ACE4_SPECIAL_WHO;
    - ace->e_flags &= ~ACE4_IDENTIFIER_GROUP;
    - return 0;
    -}
    -EXPORT_SYMBOL_GPL(richace_set_who);
    -
    -/**
    * richacl_allowed_to_who - mask flags allowed to a specific who value
    *
    * Computes the mask values allowed to a specific who value, taking
    @@ -446,10 +406,10 @@ richacl_permission(struct inode *inode, const struct richacl *acl,
    continue;
    } else if (richace_is_unix_id(ace)) {
    if (ace->e_flags & ACE4_IDENTIFIER_GROUP) {
    - if (!in_group_p(ace->u.e_id))
    + if (!in_group_p(ace->e_id))
    continue;
    } else {
    - if (current_fsuid() != ace->u.e_id)
    + if (current_fsuid() != ace->e_id)
    continue;
    }
    } else
    diff --git a/fs/richacl_xattr.c b/fs/richacl_xattr.c
    index 02a7986..3f1f557 100644
    --- a/fs/richacl_xattr.c
    +++ b/fs/richacl_xattr.c
    @@ -58,19 +58,14 @@ richacl_from_xattr(const void *value, size_t size)
    goto fail_einval;

    richacl_for_each_entry(ace, acl) {
    - const char *who = (void *)(xattr_ace + 1), *end;
    - ssize_t used = (void *)who - value;

    - if (used > size)
    - goto fail_einval;
    - end = memchr(who, 0, size - used);
    - if (!end)
    + if (((void *)xattr_ace + sizeof(*xattr_ace)) > value + size)
    goto fail_einval;

    - ace->e_type = le16_to_cpu(xattr_ace->e_type);
    - ace->e_flags = le16_to_cpu(xattr_ace->e_flags);
    - ace->e_mask = le32_to_cpu(xattr_ace->e_mask);
    - ace->u.e_id = le32_to_cpu(xattr_ace->e_id);
    + ace->e_type = le16_to_cpu(xattr_ace->e_type);
    + ace->e_flags = le32_to_cpu(xattr_ace->e_flags);
    + ace->e_mask = le32_to_cpu(xattr_ace->e_mask);
    + ace->e_id = le32_to_cpu(xattr_ace->e_id);

    if (ace->e_flags & ~ACE4_VALID_FLAGS)
    goto fail_einval;
    @@ -78,13 +73,7 @@ richacl_from_xattr(const void *value, size_t size)
    (ace->e_mask & ~ACE4_VALID_MASK))
    goto fail_einval;

    - if (who == end) {
    - if (ace->u.e_id == -1)
    - goto fail_einval; /* uid/gid needed */
    - } else if (richace_set_who(ace, who))
    - goto fail_einval;
    -
    - xattr_ace = (void *)who + ALIGN(end - who + 1, 4);
    + xattr_ace = xattr_ace + 1;
    }

    return acl;
    @@ -102,13 +91,8 @@ size_t
    richacl_xattr_size(const struct richacl *acl)
    {
    size_t size = sizeof(struct richacl_xattr);
    - const struct richace *ace;

    - richacl_for_each_entry(ace, acl) {
    - size += sizeof(struct richace_xattr) +
    - (richace_is_unix_id(ace) ? 4 :
    - ALIGN(strlen(ace->u.e_who) + 1, 4));
    - }
    + size += sizeof(struct richace_xattr) * acl->a_count;
    return size;
    }
    EXPORT_SYMBOL_GPL(richacl_xattr_size);
    @@ -136,21 +120,11 @@ richacl_to_xattr(const struct richacl *acl, void *buffer)
    xattr_ace = (void *)(xattr_acl + 1);
    richacl_for_each_entry(ace, acl) {
    xattr_ace->e_type = cpu_to_le16(ace->e_type);
    - xattr_ace->e_flags = cpu_to_le16(ace->e_flags &
    + xattr_ace->e_flags = cpu_to_le32(ace->e_flags &
    ACE4_VALID_FLAGS);
    xattr_ace->e_mask = cpu_to_le32(ace->e_mask);
    - if (richace_is_unix_id(ace)) {
    - xattr_ace->e_id = cpu_to_le32(ace->u.e_id);
    - memset(xattr_ace->e_who, 0, 4);
    - xattr_ace = (void *)xattr_ace->e_who + 4;
    - } else {
    - int sz = ALIGN(strlen(ace->u.e_who) + 1, 4);
    -
    - xattr_ace->e_id = cpu_to_le32(-1);
    - memset(xattr_ace->e_who + sz - 4, 0, 4);
    - strcpy(xattr_ace->e_who, ace->u.e_who);
    - xattr_ace = (void *)xattr_ace->e_who + sz;
    - }
    + xattr_ace->e_id = cpu_to_le32(ace->e_id);
    + xattr_ace = xattr_ace + 1;
    }
    }
    EXPORT_SYMBOL_GPL(richacl_to_xattr);
    diff --git a/include/linux/richacl.h b/include/linux/richacl.h
    index 4af6d22..e4c5156 100644
    --- a/include/linux/richacl.h
    +++ b/include/linux/richacl.h
    @@ -17,14 +17,15 @@
    #define __RICHACL_H
    #include <linux/slab.h>

    +#define ACE_OWNER_ID 130
    +#define ACE_GROUP_ID 131
    +#define ACE_EVERYONE_ID 110
    +
    struct richace {
    unsigned short e_type;
    - unsigned short e_flags;
    + unsigned int e_flags;
    unsigned int e_mask;
    - union {
    - unsigned int e_id;
    - const char *e_who;
    - } u;
    + unsigned int e_id;
    };

    struct richacl {
    @@ -74,8 +75,10 @@ struct richacl {
    /*#define ACE4_FAILED_ACCESS_ACE_FLAG 0x0020*/
    #define ACE4_IDENTIFIER_GROUP 0x0040
    #define ACE4_INHERITED_ACE 0x0080
    -/* in-memory representation only */
    -#define ACE4_SPECIAL_WHO 0x4000
    +/* richacl specific flag values */
    +#define ACE4_SPECIAL_WHO 0x80000000
    +#define ACE4_UNIXID_WHO 0x40000000
    +

    #define ACE4_VALID_FLAGS ( \
    ACE4_FILE_INHERIT_ACE | \
    @@ -83,7 +86,9 @@ struct richacl {
    ACE4_NO_PROPAGATE_INHERIT_ACE | \
    ACE4_INHERIT_ONLY_ACE | \
    ACE4_IDENTIFIER_GROUP | \
    - ACE4_INHERITED_ACE)
    + ACE4_INHERITED_ACE | \
    + ACE4_SPECIAL_WHO | \
    + ACE4_UNIXID_WHO)

    /* e_mask bitflags */
    #define ACE4_READ_DATA 0x00000001
    @@ -254,14 +259,6 @@ richacl_is_protected(const struct richacl *acl)
    return acl->a_flags & ACL4_PROTECTED;
    }

    -/*
    - * Special e_who identifiers: we use these pointer values in comparisons
    - * instead of doing a strcmp.
    - */
    -extern const char richace_owner_who[];
    -extern const char richace_group_who[];
    -extern const char richace_everyone_who[];
    -
    /**
    * richace_is_owner - check if @ace is an OWNER@ entry
    */
    @@ -269,7 +266,7 @@ static inline int
    richace_is_owner(const struct richace *ace)
    {
    return (ace->e_flags & ACE4_SPECIAL_WHO) &&
    - ace->u.e_who == richace_owner_who;
    + ace->e_id == ACE_OWNER_ID;
    }

    /**
    @@ -279,7 +276,7 @@ static inline int
    richace_is_group(const struct richace *ace)
    {
    return (ace->e_flags & ACE4_SPECIAL_WHO) &&
    - ace->u.e_who == richace_group_who;
    + ace->e_id == ACE_GROUP_ID;
    }

    /**
    @@ -289,7 +286,7 @@ static inline int
    richace_is_everyone(const struct richace *ace)
    {
    return (ace->e_flags & ACE4_SPECIAL_WHO) &&
    - ace->u.e_who == richace_everyone_who;
    + ace->e_id == ACE_EVERYONE_ID;
    }

    /**
    @@ -298,7 +295,7 @@ richace_is_everyone(const struct richace *ace)
    static inline int
    richace_is_unix_id(const struct richace *ace)
    {
    - return !(ace->e_flags & ACE4_SPECIAL_WHO);
    + return (ace->e_flags & ACE4_UNIXID_WHO);
    }

    /**
    @@ -357,7 +354,7 @@ richace_is_deny(const struct richace *ace)
    extern struct richacl *richacl_alloc(int);
    extern int richace_is_same_identifier(const struct richace *,
    const struct richace *);
    -extern int richace_set_who(struct richace *, const char *);
    +extern int richace_set_who(struct richace *, const u8*, u_int32_t);
    extern int richacl_masks_to_mode(const struct richacl *);
    extern unsigned int richacl_mode_to_mask(mode_t);
    extern unsigned int richacl_want_to_mask(unsigned int);
    diff --git a/include/linux/richacl_xattr.h b/include/linux/richacl_xattr.h
    index f79ec12..19cb61e 100644
    --- a/include/linux/richacl_xattr.h
    +++ b/include/linux/richacl_xattr.h
    @@ -22,10 +22,9 @@

    struct richace_xattr {
    __le16 e_type;
    - __le16 e_flags;
    + __le32 e_flags;
    __le32 e_mask;
    __le32 e_id;
    - char e_who[0];
    };

    struct richacl_xattr {


    \
     
     \ /
      Last update: 2011-10-21 11:43    [W:0.047 / U:0.904 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site