diff options
author | J. Bruce Fields <bfields@redhat.com> | 2014-04-02 14:59:08 -0400 |
---|---|---|
committer | J. Bruce Fields <bfields@redhat.com> | 2014-04-04 10:13:23 -0400 |
commit | 06f9cc12caa862f5bc86ebdb4f77568a4bef0167 (patch) | |
tree | 3f2df1563d5cc371e8a484006fc8a66af6713ab8 /fs | |
parent | 082f31a2169bd639785e45bf252f3d5bce0303c6 (diff) |
nfsd4: don't create unnecessary mask acl
Any setattr of the ACL attribute, even if it sets just the basic 3-ACE
ACL exactly as it was returned from a file with only mode bits, creates
a mask entry, and it is only the mask, not group, entry that is changed
by subsequent modifications of the mode bits.
So, for example, it's surprising that GROUP@ is left without read or
write permissions after a chmod 0666:
touch test
chmod 0600 test
nfs4_getfacl test
A::OWNER@:rwatTcCy
A::GROUP@:tcy
A::EVERYONE@:tcy
nfs4_getfacl test | nfs4_setfacl -S - test #
chmod 0666 test
nfs4_getfacl test
A::OWNER@:rwatTcCy
A::GROUP@:tcy
D::GROUP@:rwa
A::EVERYONE@:rwatcy
So, let's stop creating the unnecessary mask ACL.
A mask will still be created on non-trivial ACLs (ACLs with actual named
user and group ACEs), so the odd posix-acl behavior of chmod modifying
only the mask will still be left in that case; but that's consistent
with local behavior.
Reported-by: Soumya Koduri <skoduri@redhat.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
Diffstat (limited to 'fs')
-rw-r--r-- | fs/nfsd/nfs4acl.c | 13 |
1 files changed, 9 insertions, 4 deletions
diff --git a/fs/nfsd/nfs4acl.c b/fs/nfsd/nfs4acl.c index d190e33d0ec2..6f3f392d48af 100644 --- a/fs/nfsd/nfs4acl.c +++ b/fs/nfsd/nfs4acl.c | |||
@@ -542,7 +542,10 @@ posix_state_to_acl(struct posix_acl_state *state, unsigned int flags) | |||
542 | * up setting a 3-element effective posix ACL with all | 542 | * up setting a 3-element effective posix ACL with all |
543 | * permissions zero. | 543 | * permissions zero. |
544 | */ | 544 | */ |
545 | nace = 4 + state->users->n + state->groups->n; | 545 | if (!state->users->n && !state->groups->n) |
546 | nace = 3; | ||
547 | else /* Note we also include a MASK ACE in this case: */ | ||
548 | nace = 4 + state->users->n + state->groups->n; | ||
546 | pacl = posix_acl_alloc(nace, GFP_KERNEL); | 549 | pacl = posix_acl_alloc(nace, GFP_KERNEL); |
547 | if (!pacl) | 550 | if (!pacl) |
548 | return ERR_PTR(-ENOMEM); | 551 | return ERR_PTR(-ENOMEM); |
@@ -586,9 +589,11 @@ posix_state_to_acl(struct posix_acl_state *state, unsigned int flags) | |||
586 | add_to_mask(state, &state->groups->aces[i].perms); | 589 | add_to_mask(state, &state->groups->aces[i].perms); |
587 | } | 590 | } |
588 | 591 | ||
589 | pace++; | 592 | if (!state->users->n && !state->groups->n) { |
590 | pace->e_tag = ACL_MASK; | 593 | pace++; |
591 | low_mode_from_nfs4(state->mask.allow, &pace->e_perm, flags); | 594 | pace->e_tag = ACL_MASK; |
595 | low_mode_from_nfs4(state->mask.allow, &pace->e_perm, flags); | ||
596 | } | ||
592 | 597 | ||
593 | pace++; | 598 | pace++; |
594 | pace->e_tag = ACL_OTHER; | 599 | pace->e_tag = ACL_OTHER; |