| Message ID | 20230721142642.45871-1-jlayton@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | nfsd: better conform to setfacl's method for setting missing ACEs | expand |
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 >
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 > >
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);
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.