Re: [PATCH 04/29] cifs: implement get acl method

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux