[PATCH 2/2] Add SMB 2 support for getting and setting SACLs
Steve French
smfrench at gmail.com
Fri Dec 18 19:35:47 UTC 2020
Lightly updated the patch to shrink patch2 by naming a variable the
same in patch 1, to avoid a rename of a local variable in patch2. And
fixed one trivial checkpatch warning, and a trivial rebase issue.
Tentatively merged into cifs-2.6.git for-next pending more review(s)
and also the usual regression testing.
On Thu, Dec 17, 2020 at 3:00 PM Boris Protopopov <pboris at amazon.com> wrote:
>
> Fix passing of the additional security info via version
> operations. Force new open when getting SACL and avoid
> reuse of files that were previously open without
> sufficient privileges to access SACLs.
>
> Signed-off-by: Boris Protopopov <pboris at amazon.com>
> ---
>
> After further testing, I found that the security info was not being
> passed correctly to opts->get_acl and opts->get_acl_by_fid(). Also,
> it turned out that files open for read were being used to fetch
> SACL without proper privileges. This patch fixes these issues, and
> is meant to be squashed (comments dropped) with the earlier patch.
>
> fs/cifs/cifsacl.c | 10 +++++-----
> fs/cifs/smb2ops.c | 4 ++--
> fs/cifs/smb2pdu.c | 4 +++-
> fs/cifs/xattr.c | 10 ++++------
> 4 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
> index 353394d9ada8..6baa121952ce 100644
> --- a/fs/cifs/cifsacl.c
> +++ b/fs/cifs/cifsacl.c
> @@ -1245,7 +1245,7 @@ cifs_acl_to_fattr(struct cifs_sb_info *cifs_sb, struct cifs_fattr *fattr,
> int rc = 0;
> struct tcon_link *tlink = cifs_sb_tlink(cifs_sb);
> struct smb_version_operations *ops;
> - const u32 unused = 0;
> + const u32 info = 0;
>
> cifs_dbg(NOISY, "converting ACL to mode for %s\n", path);
>
> @@ -1255,9 +1255,9 @@ cifs_acl_to_fattr(struct cifs_sb_info *cifs_sb, struct cifs_fattr *fattr,
> ops = tlink_tcon(tlink)->ses->server->ops;
>
> if (pfid && (ops->get_acl_by_fid))
> - pntsd = ops->get_acl_by_fid(cifs_sb, pfid, &acllen, unused);
> + pntsd = ops->get_acl_by_fid(cifs_sb, pfid, &acllen, info);
> else if (ops->get_acl)
> - pntsd = ops->get_acl(cifs_sb, inode, path, &acllen, unused);
> + pntsd = ops->get_acl(cifs_sb, inode, path, &acllen, info);
> else {
> cifs_put_tlink(tlink);
> return -EOPNOTSUPP;
> @@ -1295,7 +1295,7 @@ id_mode_to_cifs_acl(struct inode *inode, const char *path, __u64 nmode,
> struct tcon_link *tlink = cifs_sb_tlink(cifs_sb);
> struct smb_version_operations *ops;
> bool mode_from_sid, id_from_sid;
> - const u32 unused = 0;
> + const u32 info = 0;
>
> if (IS_ERR(tlink))
> return PTR_ERR(tlink);
> @@ -1311,7 +1311,7 @@ id_mode_to_cifs_acl(struct inode *inode, const char *path, __u64 nmode,
> return -EOPNOTSUPP;
> }
>
> - pntsd = ops->get_acl(cifs_sb, inode, path, &secdesclen, unused);
> + pntsd = ops->get_acl(cifs_sb, inode, path, &secdesclen, info);
> if (IS_ERR(pntsd)) {
> rc = PTR_ERR(pntsd);
> cifs_dbg(VFS, "%s: error %d getting sec desc\n", __func__, rc);
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index d28a29728fb1..f5e198860c16 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -3315,9 +3315,9 @@ get_smb2_acl(struct cifs_sb_info *cifs_sb,
> struct cifs_ntsd *pntsd = NULL;
> struct cifsFileInfo *open_file = NULL;
>
> - if (inode)
> + if (inode && !(info & SACL_SECINFO))
> open_file = find_readable_file(CIFS_I(inode), true);
> - if (!open_file)
> + if (!open_file || (info & SACL_SECINFO))
> return get_smb2_acl_by_path(cifs_sb, path, pacllen, info);
>
> pntsd = get_smb2_acl_by_fid(cifs_sb, &open_file->fid, pacllen, info);
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index 0aeb63694306..b207e1eb6803 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -3472,8 +3472,10 @@ SMB311_posix_query_info(const unsigned int xid, struct cifs_tcon *tcon,
> int
> SMB2_query_acl(const unsigned int xid, struct cifs_tcon *tcon,
> u64 persistent_fid, u64 volatile_fid,
> - void **data, u32 *plen, u32 additional_info)
> + void **data, u32 *plen, u32 extra_info)
> {
> + __u32 additional_info = OWNER_SECINFO | GROUP_SECINFO | DACL_SECINFO |
> + extra_info;
> *plen = 0;
>
> return query_info(xid, tcon, persistent_fid, volatile_fid,
> diff --git a/fs/cifs/xattr.c b/fs/cifs/xattr.c
> index 9318a2acf4ee..6b658a1172ef 100644
> --- a/fs/cifs/xattr.c
> +++ b/fs/cifs/xattr.c
> @@ -340,21 +340,19 @@ static int cifs_xattr_get(const struct xattr_handler *handler,
> * fetch owner, DACL, and SACL if asked for full descriptor,
> * fetch owner and DACL otherwise
> */
> - u32 acllen, additional_info = 0;
> + u32 acllen, extra_info;
> struct cifs_ntsd *pacl;
>
> if (pTcon->ses->server->ops->get_acl == NULL)
> goto out; /* rc already EOPNOTSUPP */
>
> if (handler->flags == XATTR_CIFS_NTSD_FULL) {
> - additional_info = OWNER_SECINFO | GROUP_SECINFO |
> - DACL_SECINFO | SACL_SECINFO;
> + extra_info = SACL_SECINFO;
> } else {
> - additional_info = OWNER_SECINFO | GROUP_SECINFO |
> - DACL_SECINFO;
> + extra_info = 0;
> }
> pacl = pTcon->ses->server->ops->get_acl(cifs_sb,
> - inode, full_path, &acllen, additional_info);
> + inode, full_path, &acllen, extra_info);
> if (IS_ERR(pacl)) {
> rc = PTR_ERR(pacl);
> cifs_dbg(VFS, "%s: error %zd getting sec desc\n",
> --
> 2.18.4
>
--
Thanks,
Steve
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-SMB3-Add-support-for-getting-and-setting-SACLs.patch
Type: text/x-patch
Size: 15278 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20201218/f638ce40/0001-SMB3-Add-support-for-getting-and-setting-SACLs.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Add-SMB-2-support-for-getting-and-setting-SACLs.patch
Type: text/x-patch
Size: 2967 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20201218/f638ce40/0002-Add-SMB-2-support-for-getting-and-setting-SACLs.bin>
More information about the samba-technical
mailing list