Re: [PATCH] cifs: fix set of group SID via NTSD xattrs

[Steve French <smfrench@gmail.com>]

tentatively merged into cifs-2.6.git for-next pending testing

If you have ideas on how to add a test case for this, that would be
helpful (we have some cifs.ko specific tests we run in addition to the
usual xfstests and git filesystem regression tests).

On Mon, Jan 3, 2022 at 8:50 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> 'setcifsacl -g <SID>' silently fails to set the group SID on server.
>
> Actually, the bug existed since commit 438471b67963 ("CIFS: Add support
> for setting owner info, dos attributes, and create time"), but this fix
> will not apply cleanly to kernel versions <= v5.10.
>
> Fixes: a9352ee926eb ("SMB3: Add support for getting and setting SACLs")
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>
> Boris,
>
> I am a little confused from the comments in the code an emails.
> In this thread [1] you wrote that you tested "setting/getting the owner,
> DACL, and SACL...".
>
> Does it mean that you did not test setting group SID?
>
> It is also confusing that comments in the code says /* owner plus DACL */
> and /* owner/DACL/SACL */.
> Does it mean that setting group SID is not supposed to be supported?
> Or was this just an oversight?
>
> Anyway, with this patch, setcifsacl -g <SID> works as expected,
> at least when the server is samba.
>
> Thanks,
> Amir.
>
>
> [1] https://lore.kernel.org/linux-cifs/CAHhKpQ7PwgDysS3nUAA0ALLdMZqnzG6q6wL1tmn3aqOPwZbyyg@mail.gmail.com/
>
>  fs/cifs/xattr.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/fs/cifs/xattr.c b/fs/cifs/xattr.c
> index 7d8b72d67c80..9d486fbbfbbd 100644
> --- a/fs/cifs/xattr.c
> +++ b/fs/cifs/xattr.c
> @@ -175,11 +175,13 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
>                                 switch (handler->flags) {
>                                 case XATTR_CIFS_NTSD_FULL:
>                                         aclflags = (CIFS_ACL_OWNER |
> +                                                   CIFS_ACL_GROUP |
>                                                     CIFS_ACL_DACL |
>                                                     CIFS_ACL_SACL);
>                                         break;
>                                 case XATTR_CIFS_NTSD:
>                                         aclflags = (CIFS_ACL_OWNER |
> +                                                   CIFS_ACL_GROUP |
>                                                     CIFS_ACL_DACL);
>                                         break;
>                                 case XATTR_CIFS_ACL:
> --
> 2.25.1
>


-- 
Thanks,

Steve