* [PATCH] nfsd: better conform to setfacl's method for setting missing ACEs
@ 2023-07-21 14:26 Jeff Layton
2023-07-21 16:29 ` Andreas Grünbacher
0 siblings, 1 reply; 3+ messages in thread
From: Jeff Layton @ 2023-07-21 14:26 UTC (permalink / raw)
To: Chuck Lever, Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: Andreas Grünbacher, linux-nfs, linux-kernel
Andreas pointed out that the way we're setting missing ACEs doesn't
quite conform to what setfacl does. Change it to better conform to
how setfacl does this.
Cc: Andreas Grünbacher <andreas.gruenbacher@gmail.com>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
fs/nfsd/nfs4acl.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
Chuck, it might be best to fold this into the original patch, if it
looks ok.
diff --git a/fs/nfsd/nfs4acl.c b/fs/nfsd/nfs4acl.c
index 64e45551d1b6..9ec61bd0e11b 100644
--- a/fs/nfsd/nfs4acl.c
+++ b/fs/nfsd/nfs4acl.c
@@ -742,14 +742,15 @@ static int nfs4_acl_nfsv4_to_posix(struct nfs4_acl *acl,
* no owner, owning group, or others entry, a copy of the ACL
* owner, owning group, or others entry is added to the Default ACL."
*
- * If none of the requisite ACEs were set, and some explicit user or group
- * ACEs were, copy the requisite entries from the effective set.
+ * Copy any missing ACEs from the effective set.
*/
- if (!default_acl_state.valid &&
- (default_acl_state.users->n || default_acl_state.groups->n)) {
- default_acl_state.owner = effective_acl_state.owner;
- default_acl_state.group = effective_acl_state.group;
- default_acl_state.other = effective_acl_state.other;
+ if (default_acl_state.users->n || default_acl_state.groups->n) {
+ if (!(default_acl_state.valid & ACL_USER_OBJ))
+ default_acl_state.owner = effective_acl_state.owner;
+ if (!(default_acl_state.valid & ACL_GROUP_OBJ))
+ default_acl_state.group = effective_acl_state.group;
+ if (!(default_acl_state.valid & ACL_OTHER))
+ default_acl_state.other = effective_acl_state.other;
}
*pacl = posix_state_to_acl(&effective_acl_state, flags);
--
2.41.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] nfsd: better conform to setfacl's method for setting missing ACEs
2023-07-21 14:26 [PATCH] nfsd: better conform to setfacl's method for setting missing ACEs Jeff Layton
@ 2023-07-21 16:29 ` Andreas Grünbacher
2023-07-21 16:50 ` Jeff Layton
0 siblings, 1 reply; 3+ messages in thread
From: Andreas Grünbacher @ 2023-07-21 16:29 UTC (permalink / raw)
To: Jeff Layton
Cc: Chuck Lever, Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
linux-nfs, linux-kernel
Am Fr., 21. Juli 2023 um 16:26 Uhr schrieb Jeff Layton <jlayton@kernel.org>:
> Andreas pointed out that the way we're setting missing ACEs doesn't
> quite conform to what setfacl does. Change it to better conform to
> how setfacl does this.
Thanks, this looks reasonable.
Andreas
> Cc: Andreas Grünbacher <andreas.gruenbacher@gmail.com>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> fs/nfsd/nfs4acl.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> Chuck, it might be best to fold this into the original patch, if it
> looks ok.
>
> diff --git a/fs/nfsd/nfs4acl.c b/fs/nfsd/nfs4acl.c
> index 64e45551d1b6..9ec61bd0e11b 100644
> --- a/fs/nfsd/nfs4acl.c
> +++ b/fs/nfsd/nfs4acl.c
> @@ -742,14 +742,15 @@ static int nfs4_acl_nfsv4_to_posix(struct nfs4_acl *acl,
> * no owner, owning group, or others entry, a copy of the ACL
> * owner, owning group, or others entry is added to the Default ACL."
> *
> - * If none of the requisite ACEs were set, and some explicit user or group
> - * ACEs were, copy the requisite entries from the effective set.
> + * Copy any missing ACEs from the effective set.
> */
> - if (!default_acl_state.valid &&
> - (default_acl_state.users->n || default_acl_state.groups->n)) {
> - default_acl_state.owner = effective_acl_state.owner;
> - default_acl_state.group = effective_acl_state.group;
> - default_acl_state.other = effective_acl_state.other;
> + if (default_acl_state.users->n || default_acl_state.groups->n) {
> + if (!(default_acl_state.valid & ACL_USER_OBJ))
> + default_acl_state.owner = effective_acl_state.owner;
> + if (!(default_acl_state.valid & ACL_GROUP_OBJ))
> + default_acl_state.group = effective_acl_state.group;
> + if (!(default_acl_state.valid & ACL_OTHER))
> + default_acl_state.other = effective_acl_state.other;
> }
>
> *pacl = posix_state_to_acl(&effective_acl_state, flags);
> --
> 2.41.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] nfsd: better conform to setfacl's method for setting missing ACEs
2023-07-21 16:29 ` Andreas Grünbacher
@ 2023-07-21 16:50 ` Jeff Layton
0 siblings, 0 replies; 3+ messages in thread
From: Jeff Layton @ 2023-07-21 16:50 UTC (permalink / raw)
To: Andreas Grünbacher
Cc: Chuck Lever, Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
linux-nfs, linux-kernel
On Fri, 2023-07-21 at 18:29 +0200, Andreas Grünbacher wrote:
> Am Fr., 21. Juli 2023 um 16:26 Uhr schrieb Jeff Layton <jlayton@kernel.org>:
> > Andreas pointed out that the way we're setting missing ACEs doesn't
> > quite conform to what setfacl does. Change it to better conform to
> > how setfacl does this.
>
> Thanks, this looks reasonable.
>
> Andreas
>
> > Cc: Andreas Grünbacher <andreas.gruenbacher@gmail.com>
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> > fs/nfsd/nfs4acl.c | 15 ++++++++-------
> > 1 file changed, 8 insertions(+), 7 deletions(-)
> >
> > Chuck, it might be best to fold this into the original patch, if it
> > looks ok.
> >
> > diff --git a/fs/nfsd/nfs4acl.c b/fs/nfsd/nfs4acl.c
> > index 64e45551d1b6..9ec61bd0e11b 100644
> > --- a/fs/nfsd/nfs4acl.c
> > +++ b/fs/nfsd/nfs4acl.c
> > @@ -742,14 +742,15 @@ static int nfs4_acl_nfsv4_to_posix(struct nfs4_acl *acl,
> > * no owner, owning group, or others entry, a copy of the ACL
> > * owner, owning group, or others entry is added to the Default ACL."
> > *
> > - * If none of the requisite ACEs were set, and some explicit user or group
> > - * ACEs were, copy the requisite entries from the effective set.
> > + * Copy any missing ACEs from the effective set.
> > */
> > - if (!default_acl_state.valid &&
> > - (default_acl_state.users->n || default_acl_state.groups->n)) {
> > - default_acl_state.owner = effective_acl_state.owner;
> > - default_acl_state.group = effective_acl_state.group;
> > - default_acl_state.other = effective_acl_state.other;
> > + if (default_acl_state.users->n || default_acl_state.groups->n) {
I think we probably need to also do this if any "valid" bits are set.
IOW, if we explicitly set a default entry only for OWNER@, we also need
ACEs for group and other.
I'll send another revision in a bit. This time, I'll just resend an
updated patch of the original instead of a patch on a patch.
Sorry for the churn!
> > + if (!(default_acl_state.valid & ACL_USER_OBJ))
> > + default_acl_state.owner = effective_acl_state.owner;
> > + if (!(default_acl_state.valid & ACL_GROUP_OBJ))
> > + default_acl_state.group = effective_acl_state.group;
> > + if (!(default_acl_state.valid & ACL_OTHER))
> > + default_acl_state.other = effective_acl_state.other;
> > }
> >
> > *pacl = posix_state_to_acl(&effective_acl_state, flags);
> > --
> > 2.41.0
> >
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-07-21 16:50 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-21 14:26 [PATCH] nfsd: better conform to setfacl's method for setting missing ACEs Jeff Layton
2023-07-21 16:29 ` Andreas Grünbacher
2023-07-21 16:50 ` Jeff Layton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).