Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230721142642.45871-1-jlayton@kernel.org>
In-Reply-To: <20230721142642.45871-1-jlayton@kernel.org>
From:   =?UTF-8?Q?Andreas_Gr=C3=BCnbacher?= <andreas.gruenbacher@gmail.com>
Date:   Fri, 21 Jul 2023 18:29:32 +0200
Message-ID: <CAHpGcM+JCXu6TVrt+-woHCRFTKD2Dr_x6Xp0n4fw9aJHScbbMw@mail.gmail.com>
Subject: Re: [PATCH] nfsd: better conform to setfacl's method for setting
 missing ACEs
To:     Jeff Layton <jlayton@kernel.org>
Cc:     Chuck Lever <chuck.lever@oracle.com>, Neil Brown <neilb@suse.de>,
        Olga Kornievskaia <kolga@netapp.com>,
        Dai Ngo <Dai.Ngo@oracle.com>, Tom Talpey <tom@talpey.com>,
        linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

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=C3=BCnbacher <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  AC=
L
>          *  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 =3D effective_acl_state.owner;
> -               default_acl_state.group =3D effective_acl_state.group;
> -               default_acl_state.other =3D 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 =3D effective_acl_state.o=
wner;
> +               if (!(default_acl_state.valid & ACL_GROUP_OBJ))
> +                       default_acl_state.group =3D effective_acl_state.g=
roup;
> +               if (!(default_acl_state.valid & ACL_OTHER))
> +                       default_acl_state.other =3D effective_acl_state.o=
ther;
>         }
>
>         *pacl =3D posix_state_to_acl(&effective_acl_state, flags);
> --
> 2.41.0
>