Commit bec50c47 authored by J. Bruce Fields's avatar J. Bruce Fields Committed by Linus Torvalds

[PATCH] knfsd: nfsd4: acls: avoid unnecessary denies

We're inserting deny's between some ACEs in order to enforce posix draft acl
semantics which prevent permissions from accumulating across entries in an
acl.

That's fine, but we're doing that by inserting a deny after *every* allow,
which is overkill.  We shouldn't be adding them in places where they actually
make no difference.

Also replaced some helper functions for creating acl entries; I prefer just
assigning directly to the struct fields--it takes a few more lines, but the
field names provide some documentation that I think makes the result easier
understand.
Signed-off-by: default avatarJ. Bruce Fields <bfields@citi.umich.edu>
Signed-off-by: default avatarNeil Brown <neilb@suse.de>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent f43daf67
...@@ -89,12 +89,19 @@ mask_from_posix(unsigned short perm, unsigned int flags) ...@@ -89,12 +89,19 @@ mask_from_posix(unsigned short perm, unsigned int flags)
} }
static u32 static u32
deny_mask(u32 allow_mask, unsigned int flags) deny_mask_from_posix(unsigned short perm, u32 flags)
{ {
u32 ret = ~allow_mask & ~NFS4_MASK_UNSUPP; u32 mask = 0;
if (!(flags & NFS4_ACL_DIR))
ret &= ~NFS4_ACE_DELETE_CHILD; if (perm & ACL_READ)
return ret; mask |= NFS4_READ_MODE;
if (perm & ACL_WRITE)
mask |= NFS4_WRITE_MODE;
if ((perm & ACL_WRITE) && (flags & NFS4_ACL_DIR))
mask |= NFS4_ACE_DELETE_CHILD;
if (perm & ACL_EXECUTE)
mask |= NFS4_EXECUTE_MODE;
return mask;
} }
/* XXX: modify functions to return NFS errors; they're only ever /* XXX: modify functions to return NFS errors; they're only ever
...@@ -164,14 +171,51 @@ nfs4_acl_posix_to_nfsv4(struct posix_acl *pacl, struct posix_acl *dpacl, ...@@ -164,14 +171,51 @@ nfs4_acl_posix_to_nfsv4(struct posix_acl *pacl, struct posix_acl *dpacl,
return acl; return acl;
} }
struct posix_acl_summary {
unsigned short owner;
unsigned short users;
unsigned short group;
unsigned short groups;
unsigned short other;
unsigned short mask;
};
static void static void
nfs4_acl_add_pair(struct nfs4_acl *acl, int eflag, u32 mask, int whotype, summarize_posix_acl(struct posix_acl *acl, struct posix_acl_summary *pas)
uid_t owner, unsigned int flags)
{ {
nfs4_acl_add_ace(acl, NFS4_ACE_ACCESS_ALLOWED_ACE_TYPE, struct posix_acl_entry *pa, *pe;
eflag, mask, whotype, owner); pas->users = 0;
nfs4_acl_add_ace(acl, NFS4_ACE_ACCESS_DENIED_ACE_TYPE, pas->groups = 0;
eflag, deny_mask(mask, flags), whotype, owner); pas->mask = 07;
pe = acl->a_entries + acl->a_count;
FOREACH_ACL_ENTRY(pa, acl, pe) {
switch (pa->e_tag) {
case ACL_USER_OBJ:
pas->owner = pa->e_perm;
break;
case ACL_GROUP_OBJ:
pas->group = pa->e_perm;
break;
case ACL_USER:
pas->users |= pa->e_perm;
break;
case ACL_GROUP:
pas->groups |= pa->e_perm;
break;
case ACL_OTHER:
pas->other = pa->e_perm;
break;
case ACL_MASK:
pas->mask = pa->e_perm;
break;
}
}
/* We'll only care about effective permissions: */
pas->users &= pas->mask;
pas->group &= pas->mask;
pas->groups &= pas->mask;
} }
/* We assume the acl has been verified with posix_acl_valid. */ /* We assume the acl has been verified with posix_acl_valid. */
...@@ -179,30 +223,63 @@ static void ...@@ -179,30 +223,63 @@ static void
_posix_to_nfsv4_one(struct posix_acl *pacl, struct nfs4_acl *acl, _posix_to_nfsv4_one(struct posix_acl *pacl, struct nfs4_acl *acl,
unsigned int flags) unsigned int flags)
{ {
struct posix_acl_entry *pa, *pe, *group_owner_entry; struct posix_acl_entry *pa, *group_owner_entry;
u32 mask; struct nfs4_ace *ace;
unsigned short mask_mask; struct posix_acl_summary pas;
unsigned short deny;
int eflag = ((flags & NFS4_ACL_TYPE_DEFAULT) ? int eflag = ((flags & NFS4_ACL_TYPE_DEFAULT) ?
NFS4_INHERITANCE_FLAGS : 0); NFS4_INHERITANCE_FLAGS : 0);
BUG_ON(pacl->a_count < 3); BUG_ON(pacl->a_count < 3);
pe = pacl->a_entries + pacl->a_count; summarize_posix_acl(pacl, &pas);
pa = pe - 2; /* if mask entry exists, it's second from the last. */
if (pa->e_tag == ACL_MASK)
mask_mask = pa->e_perm;
else
mask_mask = S_IRWXO;
pa = pacl->a_entries; pa = pacl->a_entries;
BUG_ON(pa->e_tag != ACL_USER_OBJ); ace = acl->aces + acl->naces;
mask = mask_from_posix(pa->e_perm, flags | NFS4_ACL_OWNER);
nfs4_acl_add_pair(acl, eflag, mask, NFS4_ACL_WHO_OWNER, 0, flags); /* We could deny everything not granted by the owner: */
deny = ~pas.owner;
/*
* but it is equivalent (and simpler) to deny only what is not
* granted by later entries:
*/
deny &= pas.users | pas.group | pas.groups | pas.other;
if (deny) {
ace->type = NFS4_ACE_ACCESS_DENIED_ACE_TYPE;
ace->flag = eflag;
ace->access_mask = deny_mask_from_posix(deny, flags);
ace->whotype = NFS4_ACL_WHO_OWNER;
ace++;
acl->naces++;
}
ace->type = NFS4_ACE_ACCESS_ALLOWED_ACE_TYPE;
ace->flag = eflag;
ace->access_mask = mask_from_posix(pa->e_perm, flags | NFS4_ACL_OWNER);
ace->whotype = NFS4_ACL_WHO_OWNER;
ace++;
acl->naces++;
pa++; pa++;
while (pa->e_tag == ACL_USER) { while (pa->e_tag == ACL_USER) {
mask = mask_from_posix(pa->e_perm & mask_mask, flags); deny = ~(pa->e_perm & pas.mask);
nfs4_acl_add_pair(acl, eflag, mask, deny &= pas.groups | pas.group | pas.other;
NFS4_ACL_WHO_NAMED, pa->e_id, flags); if (deny) {
ace->type = NFS4_ACE_ACCESS_DENIED_ACE_TYPE;
ace->flag = eflag;
ace->access_mask = deny_mask_from_posix(deny, flags);
ace->whotype = NFS4_ACL_WHO_NAMED;
ace->who = pa->e_id;
ace++;
acl->naces++;
}
ace->type = NFS4_ACE_ACCESS_ALLOWED_ACE_TYPE;
ace->flag = eflag;
ace->access_mask = mask_from_posix(pa->e_perm & pas.mask,
flags);
ace->whotype = NFS4_ACL_WHO_NAMED;
ace->who = pa->e_id;
ace++;
acl->naces++;
pa++; pa++;
} }
...@@ -212,41 +289,64 @@ _posix_to_nfsv4_one(struct posix_acl *pacl, struct nfs4_acl *acl, ...@@ -212,41 +289,64 @@ _posix_to_nfsv4_one(struct posix_acl *pacl, struct nfs4_acl *acl,
/* allow ACEs */ /* allow ACEs */
group_owner_entry = pa; group_owner_entry = pa;
mask = mask_from_posix(pa->e_perm & mask_mask, flags);
nfs4_acl_add_ace(acl, NFS4_ACE_ACCESS_ALLOWED_ACE_TYPE, ace->type = NFS4_ACE_ACCESS_ALLOWED_ACE_TYPE;
NFS4_ACE_IDENTIFIER_GROUP | eflag, mask, ace->flag = eflag;
NFS4_ACL_WHO_GROUP, 0); ace->access_mask = mask_from_posix(pas.group, flags);
ace->whotype = NFS4_ACL_WHO_GROUP;
ace++;
acl->naces++;
pa++; pa++;
while (pa->e_tag == ACL_GROUP) { while (pa->e_tag == ACL_GROUP) {
mask = mask_from_posix(pa->e_perm & mask_mask, flags); ace->type = NFS4_ACE_ACCESS_ALLOWED_ACE_TYPE;
nfs4_acl_add_ace(acl, NFS4_ACE_ACCESS_ALLOWED_ACE_TYPE, ace->flag = eflag | NFS4_ACE_IDENTIFIER_GROUP;
NFS4_ACE_IDENTIFIER_GROUP | eflag, mask, ace->access_mask = mask_from_posix(pa->e_perm & pas.mask,
NFS4_ACL_WHO_NAMED, pa->e_id); flags);
ace->whotype = NFS4_ACL_WHO_NAMED;
ace->who = pa->e_id;
ace++;
acl->naces++;
pa++; pa++;
} }
/* deny ACEs */ /* deny ACEs */
pa = group_owner_entry; pa = group_owner_entry;
mask = mask_from_posix(pa->e_perm, flags);
nfs4_acl_add_ace(acl, NFS4_ACE_ACCESS_DENIED_ACE_TYPE, deny = ~pas.group & pas.other;
NFS4_ACE_IDENTIFIER_GROUP | eflag, if (deny) {
deny_mask(mask, flags), NFS4_ACL_WHO_GROUP, 0); ace->type = NFS4_ACE_ACCESS_DENIED_ACE_TYPE;
ace->flag = eflag | NFS4_ACE_IDENTIFIER_GROUP;
ace->access_mask = deny_mask_from_posix(deny, flags);
ace->whotype = NFS4_ACL_WHO_GROUP;
ace++;
acl->naces++;
}
pa++; pa++;
while (pa->e_tag == ACL_GROUP) { while (pa->e_tag == ACL_GROUP) {
mask = mask_from_posix(pa->e_perm, flags); deny = ~(pa->e_perm & pas.mask);
nfs4_acl_add_ace(acl, NFS4_ACE_ACCESS_DENIED_ACE_TYPE, deny &= pas.other;
NFS4_ACE_IDENTIFIER_GROUP | eflag, if (deny) {
deny_mask(mask, flags), NFS4_ACL_WHO_NAMED, pa->e_id); ace->type = NFS4_ACE_ACCESS_DENIED_ACE_TYPE;
ace->flag = eflag | NFS4_ACE_IDENTIFIER_GROUP;
ace->access_mask = mask_from_posix(deny, flags);
ace->whotype = NFS4_ACL_WHO_NAMED;
ace->who = pa->e_id;
ace++;
acl->naces++;
}
pa++; pa++;
} }
if (pa->e_tag == ACL_MASK) if (pa->e_tag == ACL_MASK)
pa++; pa++;
BUG_ON(pa->e_tag != ACL_OTHER); ace->type = NFS4_ACE_ACCESS_ALLOWED_ACE_TYPE;
mask = mask_from_posix(pa->e_perm, flags); ace->flag = eflag;
nfs4_acl_add_pair(acl, eflag, mask, NFS4_ACL_WHO_EVERYONE, 0, flags); ace->access_mask = mask_from_posix(pa->e_perm, flags);
ace->whotype = NFS4_ACL_WHO_EVERYONE;
acl->naces++;
} }
static void static void
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment