1. GROUP@ Allow entry doesn't have NFS4_ACE_IDENTIFIER_GROUP, This
appears to have been introduced by accident as part of commit
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=bec50c47aaf6f1f9247f1860547ab394a0802a4c
2. The group deny entries end up denying tcy even though tcy was just
allowed by the allow entry. This appears to be due to:
ace->access_mask = mask_from_posix(deny, flags);
instead of:
ace->access_mask = deny_mask_from_posix(deny, flags);
Frank
Signed-off-by: Frank Filz <[email protected]>
---
fs/nfsd/nfs4acl.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/nfsd/nfs4acl.c b/fs/nfsd/nfs4acl.c
index 54b8b41..ae37ec4 100644
--- a/fs/nfsd/nfs4acl.c
+++ b/fs/nfsd/nfs4acl.c
@@ -295,7 +295,7 @@ _posix_to_nfsv4_one(struct posix_acl *pacl, struct nfs4_acl *acl,
group_owner_entry = pa;
ace->type = NFS4_ACE_ACCESS_ALLOWED_ACE_TYPE;
- ace->flag = eflag;
+ ace->flag = eflag | NFS4_ACE_IDENTIFIER_GROUP;
ace->access_mask = mask_from_posix(pas.group, flags);
ace->whotype = NFS4_ACL_WHO_GROUP;
ace++;
@@ -335,7 +335,7 @@ _posix_to_nfsv4_one(struct posix_acl *pacl, struct nfs4_acl *acl,
if (deny) {
ace->type = NFS4_ACE_ACCESS_DENIED_ACE_TYPE;
ace->flag = eflag | NFS4_ACE_IDENTIFIER_GROUP;
- ace->access_mask = mask_from_posix(deny, flags);
+ ace->access_mask = deny_mask_from_posix(deny, flags);
ace->whotype = NFS4_ACL_WHO_NAMED;
ace->who = pa->e_id;
ace++;
--
1.5.2.2
_______________________________________________
NFSv4 mailing list
[email protected]
http://linux-nfs.org/cgi-bin/mailman/listinfo/nfsv4
On Mon, 2009-08-24 at 19:59 -0400, J. Bruce Fields wrote:
> On Fri, Aug 14, 2009 at 03:02:30PM -0700, Frank Filz wrote:
> > 1. GROUP@ Allow entry doesn't have NFS4_ACE_IDENTIFIER_GROUP, This
> > appears to have been introduced by accident as part of commit
> > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=bec50c47aaf6f1f9247f1860547ab394a0802a4c
>
> It's good to flip that bit every now and then just to keep client
> implementations on their toes....
>
> (Slightly more seriously, the 4.1 draft says "The
> ACE4_IDENTIFIER_GROUP flag MUST be ignored on entries with these
> special identifiers. When encoding entries with these special
> identifiers, the ACE4_IDENTIFIER_GROUP flag SHOULD be set to
> zero." It really shouldn't matter either way, but the point is
> that this flag is used to distinguish named users from named
> groups (since unix allows a group to have the same name as a
> user), so it doesn't really make sense to use it on a special
> identifier such as this.)
Ok, that makes sense, in that case, we probably should have this
fragment to remove the flag from the GROUP@ deny entry:
@@ -321,7 +321,7 @@ _posix_to_nfsv4_one(struct posix_acl *pacl, struct nfs4_acl *acl,
deny = ~pas.group & pas.other;
if (deny) {
ace->type = NFS4_ACE_ACCESS_DENIED_ACE_TYPE;
- ace->flag = eflag | NFS4_ACE_IDENTIFIER_GROUP;
+ ace->flag = eflag;
ace->access_mask = deny_mask_from_posix(deny, flags);
ace->whotype = NFS4_ACL_WHO_GROUP;
ace++;
On Thu, Aug 27, 2009 at 10:40:11AM -0700, Frank Filz wrote:
> On Mon, 2009-08-24 at 19:59 -0400, J. Bruce Fields wrote:
> > On Fri, Aug 14, 2009 at 03:02:30PM -0700, Frank Filz wrote:
> > > 1. GROUP@ Allow entry doesn't have NFS4_ACE_IDENTIFIER_GROUP, This
> > > appears to have been introduced by accident as part of commit
> > > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=bec50c47aaf6f1f9247f1860547ab394a0802a4c
> >
> > It's good to flip that bit every now and then just to keep client
> > implementations on their toes....
> >
> > (Slightly more seriously, the 4.1 draft says "The
> > ACE4_IDENTIFIER_GROUP flag MUST be ignored on entries with these
> > special identifiers. When encoding entries with these special
> > identifiers, the ACE4_IDENTIFIER_GROUP flag SHOULD be set to
> > zero." It really shouldn't matter either way, but the point is
> > that this flag is used to distinguish named users from named
> > groups (since unix allows a group to have the same name as a
> > user), so it doesn't really make sense to use it on a special
> > identifier such as this.)
>
> Ok, that makes sense, in that case, we probably should have this
> fragment to remove the flag from the GROUP@ deny entry:
Sure. Applied.
--b.
>
> @@ -321,7 +321,7 @@ _posix_to_nfsv4_one(struct posix_acl *pacl, struct nfs4_acl *acl,
> deny = ~pas.group & pas.other;
> if (deny) {
> ace->type = NFS4_ACE_ACCESS_DENIED_ACE_TYPE;
> - ace->flag = eflag | NFS4_ACE_IDENTIFIER_GROUP;
> + ace->flag = eflag;
> ace->access_mask = deny_mask_from_posix(deny, flags);
> ace->whotype = NFS4_ACL_WHO_GROUP;
> ace++;
>
>
On Fri, Aug 14, 2009 at 03:02:30PM -0700, Frank Filz wrote:
> 1. GROUP@ Allow entry doesn't have NFS4_ACE_IDENTIFIER_GROUP, This
> appears to have been introduced by accident as part of commit
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=bec50c47aaf6f1f9247f1860547ab394a0802a4c
It's good to flip that bit every now and then just to keep client
implementations on their toes....
(Slightly more seriously, the 4.1 draft says "The
ACE4_IDENTIFIER_GROUP flag MUST be ignored on entries with these
special identifiers. When encoding entries with these special
identifiers, the ACE4_IDENTIFIER_GROUP flag SHOULD be set to
zero." It really shouldn't matter either way, but the point is
that this flag is used to distinguish named users from named
groups (since unix allows a group to have the same name as a
user), so it doesn't really make sense to use it on a special
identifier such as this.)
> 2. The group deny entries end up denying tcy even though tcy was just
> allowed by the allow entry. This appears to be due to:
> ace->access_mask = mask_from_posix(deny, flags);
> instead of:
> ace->access_mask = deny_mask_from_posix(deny, flags);
Yup, good catch, thanks. I'll apply the second chunk from this patch
(and adjust the comment appropriately).
--b.
>
> Frank
>
> Signed-off-by: Frank Filz <[email protected]>
> ---
> fs/nfsd/nfs4acl.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfsd/nfs4acl.c b/fs/nfsd/nfs4acl.c
> index 54b8b41..ae37ec4 100644
> --- a/fs/nfsd/nfs4acl.c
> +++ b/fs/nfsd/nfs4acl.c
> @@ -295,7 +295,7 @@ _posix_to_nfsv4_one(struct posix_acl *pacl, struct nfs4_acl *acl,
> group_owner_entry = pa;
>
> ace->type = NFS4_ACE_ACCESS_ALLOWED_ACE_TYPE;
> - ace->flag = eflag;
> + ace->flag = eflag | NFS4_ACE_IDENTIFIER_GROUP;
> ace->access_mask = mask_from_posix(pas.group, flags);
> ace->whotype = NFS4_ACL_WHO_GROUP;
> ace++;
> @@ -335,7 +335,7 @@ _posix_to_nfsv4_one(struct posix_acl *pacl, struct nfs4_acl *acl,
> if (deny) {
> ace->type = NFS4_ACE_ACCESS_DENIED_ACE_TYPE;
> ace->flag = eflag | NFS4_ACE_IDENTIFIER_GROUP;
> - ace->access_mask = mask_from_posix(deny, flags);
> + ace->access_mask = deny_mask_from_posix(deny, flags);
> ace->whotype = NFS4_ACL_WHO_NAMED;
> ace->who = pa->e_id;
> ace++;
> --
> 1.5.2.2
>
>
>