Looks like the SMB1 Protocol operations for get/set posix ACL were
removed in the companion patch (in SMB3, POSIX ACLs have to be handled
by mapping from rich acls). Was this intentional or did I miss
something? I didn't see the functions for sending these over the wire
for SMB1 (which does support POSIX ACLs, not just RichACLs (SMB/NTFS
ACLs))
pSMB->SubCommand = cpu_to_le16(TRANS2_SET_PATH_INFORMATION);
pSMB->InformationLevel = cpu_to_le16(SMB_SET_POSIX_ACL);
On Thu, Sep 22, 2022 at 10:20 AM Christian Brauner <brauner@xxxxxxxxxx> wrote:
>
> The current way of setting and getting posix acls through the generic
> xattr interface is error prone and type unsafe. The vfs needs to
> interpret and fixup posix acls before storing or reporting it to
> userspace. Various hacks exist to make this work. The code is hard to
> understand and difficult to maintain in it's current form. Instead of
> making this work by hacking posix acls through xattr handlers we are
> building a dedicated posix acl api around the get and set inode
> operations. This removes a lot of hackiness and makes the codepaths
> easier to maintain. A lot of background can be found in [1].
>
> In order to build a type safe posix api around get and set acl we need
> all filesystem to implement get and set acl.
>
> So far cifs wasn't able to implement get and set acl inode operations
> because it needs access to the dentry. Now that we extended the set acl
> inode operation to take a dentry argument and added a new get acl inode
> operation that takes a dentry argument we can let cifs implement get and
> set acl inode operations.
>
> This is mostly a copy and paste of the codepaths currently used in cifs'
> posix acl xattr handler. After we have fully implemented the posix acl
> api and switched the vfs over to it, the cifs specific posix acl xattr
> handler and associated code will be removed and the code duplication
> will go away.
>
> Note, until the vfs has been switched to the new posix acl api this
> patch is a non-functional change.
>
> Link: https://lore.kernel.org/all/20220801145520.1532837-1-brauner@xxxxxxxxxx [1]
> Signed-off-by: Christian Brauner (Microsoft) <brauner@xxxxxxxxxx>
> ---
> fs/cifs/cifsacl.c | 63 +++++++++++++++
> fs/cifs/cifsfs.c | 2 +
> fs/cifs/cifsproto.h | 6 ++
> fs/cifs/cifssmb.c | 190 ++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 261 insertions(+)
>
> diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
> index fa480d62f313..06ae721ec1e7 100644
> --- a/fs/cifs/cifsacl.c
> +++ b/fs/cifs/cifsacl.c
> @@ -13,6 +13,7 @@
> #include <linux/string.h>
> #include <linux/keyctl.h>
> #include <linux/key-type.h>
> +#include <uapi/linux/posix_acl.h>
> #include <keys/user-type.h>
> #include "cifspdu.h"
> #include "cifsglob.h"
> @@ -20,6 +21,8 @@
> #include "cifsproto.h"
> #include "cifs_debug.h"
> #include "fs_context.h"
> +#include "cifs_fs_sb.h"
> +#include "cifs_unicode.h"
>
> /* security id for everyone/world system group */
> static const struct cifs_sid sid_everyone = {
> @@ -1668,3 +1671,63 @@ id_mode_to_cifs_acl(struct inode *inode, const char *path, __u64 *pnmode,
> kfree(pntsd);
> return rc;
> }
> +
> +struct posix_acl *cifs_get_acl(struct user_namespace *mnt_userns,
> + struct dentry *dentry, int type)
> +{
> +#if defined(CONFIG_CIFS_ALLOW_INSECURE_LEGACY) && defined(CONFIG_CIFS_POSIX)
> + struct posix_acl *acl = NULL;
> + ssize_t rc = -EOPNOTSUPP;
> + unsigned int xid;
> + struct super_block *sb = dentry->d_sb;
> + struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
> + struct tcon_link *tlink;
> + struct cifs_tcon *pTcon;
> + const char *full_path;
> + void *page;
> +
> + tlink = cifs_sb_tlink(cifs_sb);
> + if (IS_ERR(tlink))
> + return ERR_CAST(tlink);
> + pTcon = tlink_tcon(tlink);
> +
> + xid = get_xid();
> + page = alloc_dentry_path();
> +
> + full_path = build_path_from_dentry(dentry, page);
> + if (IS_ERR(full_path)) {
> + acl = ERR_CAST(full_path);
> + goto out;
> + }
> +
> + /* return alt name if available as pseudo attr */
> + switch (type) {
> + case ACL_TYPE_ACCESS:
> + if (sb->s_flags & SB_POSIXACL)
> + rc = cifs_do_get_acl(xid, pTcon, full_path, &acl,
> + ACL_TYPE_ACCESS,
> + cifs_sb->local_nls,
> + cifs_remap(cifs_sb));
> + break;
> +
> + case ACL_TYPE_DEFAULT:
> + if (sb->s_flags & SB_POSIXACL)
> + rc = cifs_do_get_acl(xid, pTcon, full_path, &acl,
> + ACL_TYPE_DEFAULT,
> + cifs_sb->local_nls,
> + cifs_remap(cifs_sb));
> + break;
> + }
> +
> + if (rc == -EINVAL)
> + acl = ERR_PTR(-EOPNOTSUPP);
> +
> +out:
> + free_dentry_path(page);
> + free_xid(xid);
> + cifs_put_tlink(tlink);
> + return acl;
> +#else
> + return ERR_PTR(-EOPNOTSUPP);
> +#endif
> +}
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index f54d8bf2732a..5c00d79fda99 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -1128,6 +1128,7 @@ const struct inode_operations cifs_dir_inode_ops = {
> .symlink = cifs_symlink,
> .mknod = cifs_mknod,
> .listxattr = cifs_listxattr,
> + .get_acl = cifs_get_acl,
> };
>
> const struct inode_operations cifs_file_inode_ops = {
> @@ -1136,6 +1137,7 @@ const struct inode_operations cifs_file_inode_ops = {
> .permission = cifs_permission,
> .listxattr = cifs_listxattr,
> .fiemap = cifs_fiemap,
> + .get_acl = cifs_get_acl,
> };
>
> const struct inode_operations cifs_symlink_inode_ops = {
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index 3bc94bcc7177..953fd910da70 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -225,6 +225,8 @@ extern struct cifs_ntsd *get_cifs_acl(struct cifs_sb_info *, struct inode *,
> const char *, u32 *, u32);
> extern struct cifs_ntsd *get_cifs_acl_by_fid(struct cifs_sb_info *,
> const struct cifs_fid *, u32 *, u32);
> +extern struct posix_acl *cifs_get_acl(struct user_namespace *mnt_userns,
> + struct dentry *dentry, int type);
> extern int set_cifs_acl(struct cifs_ntsd *, __u32, struct inode *,
> const char *, int);
> extern unsigned int setup_authusers_ACE(struct cifs_ace *pace);
> @@ -542,6 +544,10 @@ extern int CIFSSMBGetPosixACL(const unsigned int xid, struct cifs_tcon *tcon,
> const unsigned char *searchName,
> char *acl_inf, const int buflen, const int acl_type,
> const struct nls_table *nls_codepage, int remap_special_chars);
> +extern int cifs_do_get_acl(const unsigned int xid, struct cifs_tcon *tcon,
> + const unsigned char *searchName,
> + struct posix_acl **acl, const int acl_type,
> + const struct nls_table *nls_codepage, int remap);
> extern int CIFSSMBSetPosixACL(const unsigned int xid, struct cifs_tcon *tcon,
> const unsigned char *fileName,
> const char *local_acl, const int buflen, const int acl_type,
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index 7aa91e272027..f53d2eb100ca 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -3212,6 +3212,196 @@ CIFSSMBSetPosixACL(const unsigned int xid, struct cifs_tcon *tcon,
> return rc;
> }
>
> +/**
> + * cifs_init_posix_acl - convert ACL from cifs to POSIX ACL format
> + * @ace: POSIX ACL entry to store converted ACL into
> + * @cifs: ACL in cifs format
> + *
> + * Convert an Access Control Entry from wire format to local POSIX xattr
> + * format.
> + *
> + * Note that the @cifs_uid member is used to store both {g,u}id_t.
> + */
> +static void cifs_init_posix_acl(struct posix_acl_entry *ace,
> + struct cifs_posix_ace *cifs_ace)
> +{
> + /* u8 cifs fields do not need le conversion */
> + ace->e_perm = cpu_to_le16(cifs_ace->cifs_e_perm);
> + ace->e_tag = cpu_to_le16(cifs_ace->cifs_e_tag);
> + switch (ace->e_tag) {
> + case ACL_USER:
> + ace->e_uid = make_kuid(&init_user_ns,
> + cpu_to_le32(le64_to_cpu(cifs_ace->cifs_uid)));
> + break;
> + case ACL_GROUP:
> + ace->e_gid = make_kgid(&init_user_ns,
> + cpu_to_le32(le64_to_cpu(cifs_ace->cifs_uid)));
> + break;
> + }
> +/*
> + cifs_dbg(FYI, "perm %d tag %d id %d\n",
> + ace->e_perm, ace->e_tag, ace->e_id);
> +*/
> +
> + return;
> +}
> +
> +/**
> + * cifs_to_posix_acl - copy cifs ACL format to POSIX ACL format
> + * @acl: ACLs returned in POSIX ACL format
> + * @src: ACLs in cifs format
> + * @acl_type: type of POSIX ACL requested
> + * @size_of_data_area: size of SMB we got
> + *
> + * This function converts ACLs from cifs format to POSIX ACL format.
> + * If @acl is NULL then the size of the buffer required to store POSIX ACLs in
> + * their uapi format is returned.
> + */
> +static int cifs_to_posix_acl(struct posix_acl **acl, char *src,
> + const int acl_type, const int size_of_data_area)
> +{
> + int size = 0;
> + __u16 count;
> + struct cifs_posix_ace *pACE;
> + struct cifs_posix_acl *cifs_acl = (struct cifs_posix_acl *)src;
> + struct posix_acl *kacl = NULL;
> + struct posix_acl_entry *pa, *pe;
> +
> + if (le16_to_cpu(cifs_acl->version) != CIFS_ACL_VERSION)
> + return -EOPNOTSUPP;
> +
> + if (acl_type == ACL_TYPE_ACCESS) {
> + count = le16_to_cpu(cifs_acl->access_entry_count);
> + pACE = &cifs_acl->ace_array[0];
> + size = sizeof(struct cifs_posix_acl);
> + size += sizeof(struct cifs_posix_ace) * count;
> + /* check if we would go beyond end of SMB */
> + if (size_of_data_area < size) {
> + cifs_dbg(FYI, "bad CIFS POSIX ACL size %d vs. %d\n",
> + size_of_data_area, size);
> + return -EINVAL;
> + }
> + } else if (acl_type == ACL_TYPE_DEFAULT) {
> + count = le16_to_cpu(cifs_acl->access_entry_count);
> + size = sizeof(struct cifs_posix_acl);
> + size += sizeof(struct cifs_posix_ace) * count;
> +/* skip past access ACEs to get to default ACEs */
> + pACE = &cifs_acl->ace_array[count];
> + count = le16_to_cpu(cifs_acl->default_entry_count);
> + size += sizeof(struct cifs_posix_ace) * count;
> + /* check if we would go beyond end of SMB */
> + if (size_of_data_area < size)
> + return -EINVAL;
> + } else {
> + /* illegal type */
> + return -EINVAL;
> + }
> +
> + /* Allocate number of POSIX ACLs to store in VFS format. */
> + kacl = posix_acl_alloc(count, GFP_NOFS);
> + if (!kacl)
> + return -ENOMEM;
> +
> + FOREACH_ACL_ENTRY(pa, kacl, pe) {
> + cifs_init_posix_acl(pa, pACE);
> + pACE++;
> + }
> +
> + *acl = kacl;
> + return 0;
> +}
> +
> +int cifs_do_get_acl(const unsigned int xid, struct cifs_tcon *tcon,
> + const unsigned char *searchName, struct posix_acl **acl,
> + const int acl_type, const struct nls_table *nls_codepage,
> + int remap)
> +{
> +/* SMB_QUERY_POSIX_ACL */
> + TRANSACTION2_QPI_REQ *pSMB = NULL;
> + TRANSACTION2_QPI_RSP *pSMBr = NULL;
> + int rc = 0;
> + int bytes_returned;
> + int name_len;
> + __u16 params, byte_count;
> +
> + cifs_dbg(FYI, "In GetPosixACL (Unix) for path %s\n", searchName);
> +
> +queryAclRetry:
> + rc = smb_init(SMB_COM_TRANSACTION2, 15, tcon, (void **) &pSMB,
> + (void **) &pSMBr);
> + if (rc)
> + return rc;
> +
> + if (pSMB->hdr.Flags2 & SMBFLG2_UNICODE) {
> + name_len =
> + cifsConvertToUTF16((__le16 *) pSMB->FileName,
> + searchName, PATH_MAX, nls_codepage,
> + remap);
> + name_len++; /* trailing null */
> + name_len *= 2;
> + pSMB->FileName[name_len] = 0;
> + pSMB->FileName[name_len+1] = 0;
> + } else {
> + name_len = copy_path_name(pSMB->FileName, searchName);
> + }
> +
> + params = 2 /* level */ + 4 /* rsrvd */ + name_len /* incl null */ ;
> + pSMB->TotalDataCount = 0;
> + pSMB->MaxParameterCount = cpu_to_le16(2);
> + /* BB find exact max data count below from sess structure BB */
> + pSMB->MaxDataCount = cpu_to_le16(4000);
> + pSMB->MaxSetupCount = 0;
> + pSMB->Reserved = 0;
> + pSMB->Flags = 0;
> + pSMB->Timeout = 0;
> + pSMB->Reserved2 = 0;
> + pSMB->ParameterOffset = cpu_to_le16(
> + offsetof(struct smb_com_transaction2_qpi_req,
> + InformationLevel) - 4);
> + pSMB->DataCount = 0;
> + pSMB->DataOffset = 0;
> + pSMB->SetupCount = 1;
> + pSMB->Reserved3 = 0;
> + pSMB->SubCommand = cpu_to_le16(TRANS2_QUERY_PATH_INFORMATION);
> + byte_count = params + 1 /* pad */ ;
> + pSMB->TotalParameterCount = cpu_to_le16(params);
> + pSMB->ParameterCount = pSMB->TotalParameterCount;
> + pSMB->InformationLevel = cpu_to_le16(SMB_QUERY_POSIX_ACL);
> + pSMB->Reserved4 = 0;
> + inc_rfc1001_len(pSMB, byte_count);
> + pSMB->ByteCount = cpu_to_le16(byte_count);
> +
> + rc = SendReceive(xid, tcon->ses, (struct smb_hdr *) pSMB,
> + (struct smb_hdr *) pSMBr, &bytes_returned, 0);
> + cifs_stats_inc(&tcon->stats.cifs_stats.num_acl_get);
> + if (rc) {
> + cifs_dbg(FYI, "Send error in Query POSIX ACL = %d\n", rc);
> + } else {
> + /* decode response */
> +
> + rc = validate_t2((struct smb_t2_rsp *)pSMBr);
> + /* BB also check enough total bytes returned */
> + if (rc || get_bcc(&pSMBr->hdr) < 2)
> + rc = -EIO; /* bad smb */
> + else {
> + __u16 data_offset = le16_to_cpu(pSMBr->t2.DataOffset);
> + __u16 count = le16_to_cpu(pSMBr->t2.DataCount);
> + rc = cifs_to_posix_acl(acl,
> + (char *)&pSMBr->hdr.Protocol+data_offset,
> + acl_type, count);
> + }
> + }
> + cifs_buf_release(pSMB);
> + /*
> + * The else branch after SendReceive() doesn't return EAGAIN so if we
> + * allocated @acl in cifs_to_posix_acl() we are guaranteed to return
> + * here and don't leak POSIX ACLs.
> + */
> + if (rc == -EAGAIN)
> + goto queryAclRetry;
> + return rc;
> +}
> +
> int
> CIFSGetExtAttr(const unsigned int xid, struct cifs_tcon *tcon,
> const int netfid, __u64 *pExtAttrBits, __u64 *pMask)
> --
> 2.34.1
>
--
Thanks,
Steve
