[PATCH] Add support for getting and setting SACLs

[PATCH] Add support for getting and setting SACLs

[Boris Protopopov]

Add SYSTEM_SECURITY access flag and use with smb2 when opening
files for getting/setting SACLs. Add "system.cifs_ntsd_full"
extended attribute to allow user-space access to the functionality.
Avoid multiple server calls when setting owner, DACL, and SACL.

Signed-off-by: Boris Protopopov <pboris@amazon.com>
---
 fs/cifs/cifsacl.c   | 15 ++++----
 fs/cifs/cifsglob.h  |  4 +--
 fs/cifs/cifspdu.h   |  2 ++
 fs/cifs/cifsproto.h |  4 +--
 fs/cifs/smb2ops.c   | 31 ++++++++++-------
 fs/cifs/smb2pdu.c   |  5 ++-
 fs/cifs/smb2proto.h |  4 +--
 fs/cifs/xattr.c     | 83 +++++++++++++++++++++++++++++++++------------
 8 files changed, 100 insertions(+), 48 deletions(-)

diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
index 23b21e943652..353394d9ada8 100644
--- a/fs/cifs/cifsacl.c
+++ b/fs/cifs/cifsacl.c
@@ -1101,7 +1101,8 @@ static int build_sec_desc(struct cifs_ntsd *pntsd, struct cifs_ntsd *pnntsd,
 }
 
 struct cifs_ntsd *get_cifs_acl_by_fid(struct cifs_sb_info *cifs_sb,
-		const struct cifs_fid *cifsfid, u32 *pacllen)
+				      const struct cifs_fid *cifsfid, u32 *pacllen,
+				      u32 unused __attribute__((unused)))
 {
 	struct cifs_ntsd *pntsd = NULL;
 	unsigned int xid;
@@ -1169,7 +1170,7 @@ static struct cifs_ntsd *get_cifs_acl_by_path(struct cifs_sb_info *cifs_sb,
 /* Retrieve an ACL from the server */
 struct cifs_ntsd *get_cifs_acl(struct cifs_sb_info *cifs_sb,
 				      struct inode *inode, const char *path,
-				      u32 *pacllen)
+			       u32 *pacllen, u32 unused)
 {
 	struct cifs_ntsd *pntsd = NULL;
 	struct cifsFileInfo *open_file = NULL;
@@ -1179,7 +1180,7 @@ struct cifs_ntsd *get_cifs_acl(struct cifs_sb_info *cifs_sb,
 	if (!open_file)
 		return get_cifs_acl_by_path(cifs_sb, path, pacllen);
 
-	pntsd = get_cifs_acl_by_fid(cifs_sb, &open_file->fid, pacllen);
+	pntsd = get_cifs_acl_by_fid(cifs_sb, &open_file->fid, pacllen, unused);
 	cifsFileInfo_put(open_file);
 	return pntsd;
 }
@@ -1244,6 +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;
 
 	cifs_dbg(NOISY, "converting ACL to mode for %s\n", path);
 
@@ -1253,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);
+		pntsd = ops->get_acl_by_fid(cifs_sb, pfid, &acllen, unused);
 	else if (ops->get_acl)
-		pntsd = ops->get_acl(cifs_sb, inode, path, &acllen);
+		pntsd = ops->get_acl(cifs_sb, inode, path, &acllen, unused);
 	else {
 		cifs_put_tlink(tlink);
 		return -EOPNOTSUPP;
@@ -1293,6 +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;
 
 	if (IS_ERR(tlink))
 		return PTR_ERR(tlink);
@@ -1308,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);
+	pntsd = ops->get_acl(cifs_sb, inode, path, &secdesclen, unused);
 	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/cifsglob.h b/fs/cifs/cifsglob.h
index 484ec2d8c5c9..203623b023b4 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -456,9 +456,9 @@ struct smb_version_operations {
 			const char *, const void *, const __u16,
 			const struct nls_table *, struct cifs_sb_info *);
 	struct cifs_ntsd * (*get_acl)(struct cifs_sb_info *, struct inode *,
-			const char *, u32 *);
+			const char *, u32 *, u32);
 	struct cifs_ntsd * (*get_acl_by_fid)(struct cifs_sb_info *,
-			const struct cifs_fid *, u32 *);
+			const struct cifs_fid *, u32 *, u32);
 	int (*set_acl)(struct cifs_ntsd *, __u32, struct inode *, const char *,
 			int);
 	/* writepages retry size */
diff --git a/fs/cifs/cifspdu.h b/fs/cifs/cifspdu.h
index 593d826820c3..7d5502ed2220 100644
--- a/fs/cifs/cifspdu.h
+++ b/fs/cifs/cifspdu.h
@@ -240,6 +240,8 @@
 #define SYNCHRONIZE           0x00100000  /* The file handle can waited on to */
 					  /* synchronize with the completion  */
 					  /* of an input/output request       */
+#define SYSTEM_SECURITY       0x01000000  /* The system access control list   */
+					  /* can be read and changed          */
 #define GENERIC_ALL           0x10000000
 #define GENERIC_EXECUTE       0x20000000
 #define GENERIC_WRITE         0x40000000
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 24c6f36177ba..8e43628b4dd7 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -218,9 +218,9 @@ extern int cifs_acl_to_fattr(struct cifs_sb_info *cifs_sb,
 extern int id_mode_to_cifs_acl(struct inode *inode, const char *path, __u64,
 					kuid_t, kgid_t);
 extern struct cifs_ntsd *get_cifs_acl(struct cifs_sb_info *, struct inode *,
-					const char *, u32 *);
+				      const char *, u32 *, u32);
 extern struct cifs_ntsd *get_cifs_acl_by_fid(struct cifs_sb_info *,
-						const struct cifs_fid *, u32 *);
+				const struct cifs_fid *, u32 *, u32);
 extern int set_cifs_acl(struct cifs_ntsd *, __u32, struct inode *,
 				const char *, int);
 extern unsigned int setup_authusers_ACE(struct cifs_ace *pace);
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 504766cb6c19..96965c97e141 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -3163,7 +3163,7 @@ smb2_query_reparse_tag(const unsigned int xid, struct cifs_tcon *tcon,
 
 static struct cifs_ntsd *
 get_smb2_acl_by_fid(struct cifs_sb_info *cifs_sb,
-		const struct cifs_fid *cifsfid, u32 *pacllen)
+		    const struct cifs_fid *cifsfid, u32 *pacllen, u32 info)
 {
 	struct cifs_ntsd *pntsd = NULL;
 	unsigned int xid;
@@ -3177,7 +3177,8 @@ get_smb2_acl_by_fid(struct cifs_sb_info *cifs_sb,
 	cifs_dbg(FYI, "trying to get acl\n");
 
 	rc = SMB2_query_acl(xid, tlink_tcon(tlink), cifsfid->persistent_fid,
-			    cifsfid->volatile_fid, (void **)&pntsd, pacllen);
+			    cifsfid->volatile_fid, (void **)&pntsd, pacllen,
+			    info);
 	free_xid(xid);
 
 	cifs_put_tlink(tlink);
@@ -3191,7 +3192,7 @@ get_smb2_acl_by_fid(struct cifs_sb_info *cifs_sb,
 
 static struct cifs_ntsd *
 get_smb2_acl_by_path(struct cifs_sb_info *cifs_sb,
-		const char *path, u32 *pacllen)
+		     const char *path, u32 *pacllen, u32 info)
 {
 	struct cifs_ntsd *pntsd = NULL;
 	u8 oplock = SMB2_OPLOCK_LEVEL_NONE;
@@ -3229,12 +3230,16 @@ get_smb2_acl_by_path(struct cifs_sb_info *cifs_sb,
 	oparms.fid = &fid;
 	oparms.reconnect = false;
 
+	if (info & SACL_SECINFO)
+		oparms.desired_access |= SYSTEM_SECURITY;
+
 	rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, NULL, NULL,
 		       NULL);
 	kfree(utf16_path);
 	if (!rc) {
 		rc = SMB2_query_acl(xid, tlink_tcon(tlink), fid.persistent_fid,
-			    fid.volatile_fid, (void **)&pntsd, pacllen);
+				    fid.volatile_fid, (void **)&pntsd, pacllen,
+				    info);
 		SMB2_close(xid, tcon, fid.persistent_fid, fid.volatile_fid);
 	}
 
@@ -3268,10 +3273,12 @@ set_smb2_acl(struct cifs_ntsd *pnntsd, __u32 acllen,
 	tcon = tlink_tcon(tlink);
 	xid = get_xid();
 
-	if (aclflag == CIFS_ACL_OWNER || aclflag == CIFS_ACL_GROUP)
-		access_flags = WRITE_OWNER;
-	else
-		access_flags = WRITE_DAC;
+	if (aclflag & CIFS_ACL_OWNER || aclflag & CIFS_ACL_GROUP)
+		access_flags |= WRITE_OWNER;
+	if (aclflag & CIFS_ACL_SACL)
+		access_flags |= SYSTEM_SECURITY;
+	if (aclflag & CIFS_ACL_DACL)
+		access_flags |= WRITE_DAC;
 
 	utf16_path = cifs_convert_path_to_utf16(path, cifs_sb);
 	if (!utf16_path) {
@@ -3305,8 +3312,8 @@ set_smb2_acl(struct cifs_ntsd *pnntsd, __u32 acllen,
 /* Retrieve an ACL from the server */
 static struct cifs_ntsd *
 get_smb2_acl(struct cifs_sb_info *cifs_sb,
-				      struct inode *inode, const char *path,
-				      u32 *pacllen)
+	     struct inode *inode, const char *path,
+	     u32 *pacllen, u32 info)
 {
 	struct cifs_ntsd *pntsd = NULL;
 	struct cifsFileInfo *open_file = NULL;
@@ -3314,9 +3321,9 @@ get_smb2_acl(struct cifs_sb_info *cifs_sb,
 	if (inode)
 		open_file = find_readable_file(CIFS_I(inode), true);
 	if (!open_file)
-		return get_smb2_acl_by_path(cifs_sb, path, pacllen);
+		return get_smb2_acl_by_path(cifs_sb, path, pacllen, info);
 
-	pntsd = get_smb2_acl_by_fid(cifs_sb, &open_file->fid, pacllen);
+	pntsd = get_smb2_acl_by_fid(cifs_sb, &open_file->fid, pacllen, info);
 	cifsFileInfo_put(open_file);
 	return pntsd;
 }
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 445e80862865..0aeb63694306 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -3471,10 +3471,9 @@ 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)
+	       u64 persistent_fid, u64 volatile_fid,
+	       void **data, u32 *plen, u32 additional_info)
 {
-	__u32 additional_info = OWNER_SECINFO | GROUP_SECINFO | DACL_SECINFO;
 	*plen = 0;
 
 	return query_info(xid, tcon, persistent_fid, volatile_fid,
diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
index d4110447ee3a..9565e27681a5 100644
--- a/fs/cifs/smb2proto.h
+++ b/fs/cifs/smb2proto.h
@@ -200,8 +200,8 @@ extern int SMB2_query_info_init(struct cifs_tcon *tcon,
 				size_t input_len, void *input);
 extern void SMB2_query_info_free(struct smb_rqst *rqst);
 extern int SMB2_query_acl(const unsigned int xid, struct cifs_tcon *tcon,
-			   u64 persistent_file_id, u64 volatile_file_id,
-			   void **data, unsigned int *plen);
+			  u64 persistent_file_id, u64 volatile_file_id,
+			  void **data, unsigned int *plen, u32 info);
 extern int SMB2_get_srv_num(const unsigned int xid, struct cifs_tcon *tcon,
 			    u64 persistent_fid, u64 volatile_fid,
 			    __le64 *uniqueid);
diff --git a/fs/cifs/xattr.c b/fs/cifs/xattr.c
index b8299173ea7e..9318a2acf4ee 100644
--- a/fs/cifs/xattr.c
+++ b/fs/cifs/xattr.c
@@ -34,6 +34,7 @@
 #define MAX_EA_VALUE_SIZE CIFSMaxBufSize
 #define CIFS_XATTR_CIFS_ACL "system.cifs_acl" /* DACL only */
 #define CIFS_XATTR_CIFS_NTSD "system.cifs_ntsd" /* owner plus DACL */
+#define CIFS_XATTR_CIFS_NTSD_FULL "system.cifs_ntsd_full" /* owner/DACL/SACL */
 #define CIFS_XATTR_ATTRIB "cifs.dosattrib"  /* full name: user.cifs.dosattrib */
 #define CIFS_XATTR_CREATETIME "cifs.creationtime"  /* user.cifs.creationtime */
 /*
@@ -43,12 +44,13 @@
  */
 #define SMB3_XATTR_CIFS_ACL "system.smb3_acl" /* DACL only */
 #define SMB3_XATTR_CIFS_NTSD "system.smb3_ntsd" /* owner plus DACL */
+#define SMB3_XATTR_CIFS_NTSD_FULL "system.smb3_ntsd_full" /* owner/DACL/SACL */
 #define SMB3_XATTR_ATTRIB "smb3.dosattrib"  /* full name: user.smb3.dosattrib */
 #define SMB3_XATTR_CREATETIME "smb3.creationtime"  /* user.smb3.creationtime */
 /* BB need to add server (Samba e.g) support for security and trusted prefix */
 
 enum { XATTR_USER, XATTR_CIFS_ACL, XATTR_ACL_ACCESS, XATTR_ACL_DEFAULT,
-	XATTR_CIFS_NTSD };
+	XATTR_CIFS_NTSD, XATTR_CIFS_NTSD_FULL };
 
 static int cifs_attrib_set(unsigned int xid, struct cifs_tcon *pTcon,
 			   struct inode *inode, char *full_path,
@@ -164,7 +166,8 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
 		break;
 
 	case XATTR_CIFS_ACL:
-	case XATTR_CIFS_NTSD: {
+	case XATTR_CIFS_NTSD:
+	case XATTR_CIFS_NTSD_FULL: {
 		struct cifs_ntsd *pacl;
 
 		if (!value)
@@ -174,23 +177,27 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
 			rc = -ENOMEM;
 		} else {
 			memcpy(pacl, value, size);
-			if (value &&
-			    pTcon->ses->server->ops->set_acl) {
+			if (pTcon->ses->server->ops->set_acl) {
+				int aclflags = 0;
 				rc = 0;
-				if (handler->flags == XATTR_CIFS_NTSD) {
-					/* set owner and DACL */
-					rc = pTcon->ses->server->ops->set_acl(
-							pacl, size, inode,
-							full_path,
-							CIFS_ACL_OWNER);
-				}
-				if (rc == 0) {
-					/* set DACL */
-					rc = pTcon->ses->server->ops->set_acl(
-							pacl, size, inode,
-							full_path,
-							CIFS_ACL_DACL);
+
+				switch (handler->flags) {
+				case XATTR_CIFS_NTSD_FULL:
+					aclflags = (CIFS_ACL_OWNER |
+						    CIFS_ACL_DACL |
+						    CIFS_ACL_SACL);
+					break;
+				case XATTR_CIFS_NTSD:
+					aclflags = (CIFS_ACL_OWNER |
+						    CIFS_ACL_DACL);
+					break;
+				case XATTR_CIFS_ACL:
+				default:
+					aclflags = CIFS_ACL_DACL;
 				}
+
+				rc = pTcon->ses->server->ops->set_acl(pacl,
+					size, inode, full_path, aclflags);
 			} else {
 				rc = -EOPNOTSUPP;
 			}
@@ -327,16 +334,27 @@ static int cifs_xattr_get(const struct xattr_handler *handler,
 		break;
 
 	case XATTR_CIFS_ACL:
-	case XATTR_CIFS_NTSD: {
-		/* the whole ntsd is fetched regardless */
-		u32 acllen;
+	case XATTR_CIFS_NTSD:
+	case XATTR_CIFS_NTSD_FULL: {
+		/*
+		 * fetch owner, DACL, and SACL if asked for full descriptor,
+		 * fetch owner and DACL otherwise
+		 */
+		u32 acllen, additional_info = 0;
 		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;
+		} else {
+			additional_info = OWNER_SECINFO | GROUP_SECINFO |
+				DACL_SECINFO;
+		}
 		pacl = pTcon->ses->server->ops->get_acl(cifs_sb,
-				inode, full_path, &acllen);
+				inode, full_path, &acllen, additional_info);
 		if (IS_ERR(pacl)) {
 			rc = PTR_ERR(pacl);
 			cifs_dbg(VFS, "%s: error %zd getting sec desc\n",
@@ -486,6 +504,27 @@ static const struct xattr_handler smb3_ntsd_xattr_handler = {
 	.set = cifs_xattr_set,
 };
 
+static const struct xattr_handler cifs_cifs_ntsd_full_xattr_handler = {
+	.name = CIFS_XATTR_CIFS_NTSD_FULL,
+	.flags = XATTR_CIFS_NTSD_FULL,
+	.get = cifs_xattr_get,
+	.set = cifs_xattr_set,
+};
+
+/*
+ * Although this is just an alias for the above, need to move away from
+ * confusing users and using the 20 year old term 'cifs' when it is no
+ * longer secure and was replaced by SMB2/SMB3 a long time ago, and
+ * SMB3 and later are highly secure.
+ */
+static const struct xattr_handler smb3_ntsd_full_xattr_handler = {
+	.name = SMB3_XATTR_CIFS_NTSD_FULL,
+	.flags = XATTR_CIFS_NTSD_FULL,
+	.get = cifs_xattr_get,
+	.set = cifs_xattr_set,
+};
+
+
 static const struct xattr_handler cifs_posix_acl_access_xattr_handler = {
 	.name = XATTR_NAME_POSIX_ACL_ACCESS,
 	.flags = XATTR_ACL_ACCESS,
@@ -507,6 +546,8 @@ const struct xattr_handler *cifs_xattr_handlers[] = {
 	&smb3_acl_xattr_handler, /* alias for above since avoiding "cifs" */
 	&cifs_cifs_ntsd_xattr_handler,
 	&smb3_ntsd_xattr_handler, /* alias for above since avoiding "cifs" */
+	&cifs_cifs_ntsd_full_xattr_handler,
+	&smb3_ntsd_full_xattr_handler, /* alias for above since avoiding "cifs" */
 	&cifs_posix_acl_access_xattr_handler,
 	&cifs_posix_acl_default_xattr_handler,
 	NULL
-- 
2.23.3

Re: [PATCH] Add support for getting and setting SACLs

[Boris Protopopov]

Hello,
I am checking in to see if anyone had a chance to take a look at this
patch. I would appreciate any feedback.
Thanks!

On Tue, Oct 27, 2020 at 5:01 PM Boris Protopopov via samba-technical
<samba-technical@lists.samba.org> wrote:
>
> Add SYSTEM_SECURITY access flag and use with smb2 when opening
> files for getting/setting SACLs. Add "system.cifs_ntsd_full"
> extended attribute to allow user-space access to the functionality.
> Avoid multiple server calls when setting owner, DACL, and SACL.
>
> Signed-off-by: Boris Protopopov <pboris@amazon.com>
> ---
...

Re: [PATCH] Add support for getting and setting SACLs

[Shyam Prasad N]

Hi Boris,

At a high level, the changes look good to me. Will go through the
changes in more detail tomorrow morning.

On a related note, we may need more test coverage in this area.
Can you please share your testing output against your changes?
Or even better, if you can contribute some xfstests for this use case:
https://wiki.samba.org/index.php/Xfstesting-cifs

Regards,
Shyam

On Wed, Dec 2, 2020 at 12:24 AM Boris Protopopov
<boris.v.protopopov@gmail.com> wrote:
>
> Hello,
> I am checking in to see if anyone had a chance to take a look at this
> patch. I would appreciate any feedback.
> Thanks!
>
> On Tue, Oct 27, 2020 at 5:01 PM Boris Protopopov via samba-technical
> <samba-technical@lists.samba.org> wrote:
> >
> > Add SYSTEM_SECURITY access flag and use with smb2 when opening
> > files for getting/setting SACLs. Add "system.cifs_ntsd_full"
> > extended attribute to allow user-space access to the functionality.
> > Avoid multiple server calls when setting owner, DACL, and SACL.
> >
> > Signed-off-by: Boris Protopopov <pboris@amazon.com>
> > ---
> ...



-- 
-Shyam

Re: [PATCH] Add support for getting and setting SACLs

[Boris Protopopov]

Hello, Shyam,

sorry for the delayed reply and thanks for looking at this patch. Yes,
the testing was done using the extended versions of
getcifsacl/setcifsacl (added setting owner and SACL support), the
patch for that posted recently via samba-technical (message ID
<20201120214918.12517-1-pboris@amazon.com>). I have tested
setting/getting the owner, DACL, and SACL, for all the DACL/SACL flags
(-a, -D, -M, -S), SACL type SYSTEM_AUDIT. This testing was done
against 5.10.0-rc1 and 4.14.203 (the latter required porting the
user-space patch). I believe this testing has fully exercised the code
changes in question.

I will look at contributing to the fsxtesting-cifs code, but I think
the setcifsacl/getcifsact patch that enables easy access to the
descriptor components is a pre-requisite for such contributions.

Thanks!


On Wed, Dec 2, 2020 at 5:43 AM Shyam Prasad N <nspmangalore@gmail.com> wrote:
>
> Hi Boris,
>
> At a high level, the changes look good to me. Will go through the
> changes in more detail tomorrow morning.
>
> On a related note, we may need more test coverage in this area.
> Can you please share your testing output against your changes?
> Or even better, if you can contribute some xfstests for this use case:
> https://wiki.samba.org/index.php/Xfstesting-cifs
>
> Regards,
> Shyam
>
> On Wed, Dec 2, 2020 at 12:24 AM Boris Protopopov
> <boris.v.protopopov@gmail.com> wrote:
> >
> > Hello,
> > I am checking in to see if anyone had a chance to take a look at this
> > patch. I would appreciate any feedback.
> > Thanks!
> >
> > On Tue, Oct 27, 2020 at 5:01 PM Boris Protopopov via samba-technical
> > <samba-technical@lists.samba.org> wrote:
> > >
> > > Add SYSTEM_SECURITY access flag and use with smb2 when opening
> > > files for getting/setting SACLs. Add "system.cifs_ntsd_full"
> > > extended attribute to allow user-space access to the functionality.
> > > Avoid multiple server calls when setting owner, DACL, and SACL.
> > >
> > > Signed-off-by: Boris Protopopov <pboris@amazon.com>
> > > ---
> > ...
>
>
>
> --
> -Shyam

Re: [PATCH] Add support for getting and setting SACLs

[Pavel Shilovsky]

Hi Boris,

Are you talking about this patch "[PATCH] Extend cifs acl utilities to
handle SACLs"?

Just for the future, I am trying to monitor the samba-dev mailing list
but if you would like to get the fastest response to your patches then
please include me directly or at least the linux-cifs mailing list.

I stage pending patches in the "next" branch on my github tree, so,
will include the one above.

https://github.com/piastry/cifs-utils/commits/next

--
Best regards,
Pavel Shilovsky

пн, 7 дек. 2020 г. в 07:28, Boris Protopopov <boris.v.protopopov@gmail.com>:
>
> Hello, Shyam,
>
> sorry for the delayed reply and thanks for looking at this patch. Yes,
> the testing was done using the extended versions of
> getcifsacl/setcifsacl (added setting owner and SACL support), the
> patch for that posted recently via samba-technical (message ID
> <20201120214918.12517-1-pboris@amazon.com>). I have tested
> setting/getting the owner, DACL, and SACL, for all the DACL/SACL flags
> (-a, -D, -M, -S), SACL type SYSTEM_AUDIT. This testing was done
> against 5.10.0-rc1 and 4.14.203 (the latter required porting the
> user-space patch). I believe this testing has fully exercised the code
> changes in question.
>
> I will look at contributing to the fsxtesting-cifs code, but I think
> the setcifsacl/getcifsact patch that enables easy access to the
> descriptor components is a pre-requisite for such contributions.
>
> Thanks!
>
>
> On Wed, Dec 2, 2020 at 5:43 AM Shyam Prasad N <nspmangalore@gmail.com> wrote:
> >
> > Hi Boris,
> >
> > At a high level, the changes look good to me. Will go through the
> > changes in more detail tomorrow morning.
> >
> > On a related note, we may need more test coverage in this area.
> > Can you please share your testing output against your changes?
> > Or even better, if you can contribute some xfstests for this use case:
> > https://wiki.samba.org/index.php/Xfstesting-cifs
> >
> > Regards,
> > Shyam
> >
> > On Wed, Dec 2, 2020 at 12:24 AM Boris Protopopov
> > <boris.v.protopopov@gmail.com> wrote:
> > >
> > > Hello,
> > > I am checking in to see if anyone had a chance to take a look at this
> > > patch. I would appreciate any feedback.
> > > Thanks!
> > >
> > > On Tue, Oct 27, 2020 at 5:01 PM Boris Protopopov via samba-technical
> > > <samba-technical@lists.samba.org> wrote:
> > > >
> > > > Add SYSTEM_SECURITY access flag and use with smb2 when opening
> > > > files for getting/setting SACLs. Add "system.cifs_ntsd_full"
> > > > extended attribute to allow user-space access to the functionality.
> > > > Avoid multiple server calls when setting owner, DACL, and SACL.
> > > >
> > > > Signed-off-by: Boris Protopopov <pboris@amazon.com>
> > > > ---
> > > ...
> >
> >
> >
> > --
> > -Shyam

Re: [PATCH] Add support for getting and setting SACLs

[Boris Protopopov]

HI, Pavel,

yes, thanks for the pointer, will include in the future.

Boris.

On Mon, Dec 7, 2020 at 2:05 PM Pavel Shilovsky <piastryyy@gmail.com> wrote:
>
> Hi Boris,
>
> Are you talking about this patch "[PATCH] Extend cifs acl utilities to
> handle SACLs"?
>
> Just for the future, I am trying to monitor the samba-dev mailing list
> but if you would like to get the fastest response to your patches then
> please include me directly or at least the linux-cifs mailing list.
>
> I stage pending patches in the "next" branch on my github tree, so,
> will include the one above.
>
> https://github.com/piastry/cifs-utils/commits/next
>
> --
> Best regards,
> Pavel Shilovsky
>
> пн, 7 дек. 2020 г. в 07:28, Boris Protopopov <boris.v.protopopov@gmail.com>:
> >
> > Hello, Shyam,
> >
> > sorry for the delayed reply and thanks for looking at this patch. Yes,
> > the testing was done using the extended versions of
> > getcifsacl/setcifsacl (added setting owner and SACL support), the
> > patch for that posted recently via samba-technical (message ID
> > <20201120214918.12517-1-pboris@amazon.com>). I have tested
> > setting/getting the owner, DACL, and SACL, for all the DACL/SACL flags
> > (-a, -D, -M, -S), SACL type SYSTEM_AUDIT. This testing was done
> > against 5.10.0-rc1 and 4.14.203 (the latter required porting the
> > user-space patch). I believe this testing has fully exercised the code
> > changes in question.
> >
> > I will look at contributing to the fsxtesting-cifs code, but I think
> > the setcifsacl/getcifsact patch that enables easy access to the
> > descriptor components is a pre-requisite for such contributions.
> >
> > Thanks!
> >
> >

[PATCH 2/2] Add SMB 2 support for getting and setting SACLs

[Boris Protopopov]

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@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

Re: [PATCH 2/2] Add SMB 2 support for getting and setting SACLs

[Shyam Prasad N]

Minor comments inline...

On Fri, Dec 18, 2020 at 2:30 AM Boris Protopopov <pboris@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@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);
Why not have an if (info & SACL_SECINFO) return
get_smb2_acl_by_path... right at the beginning?
Looks cleaner that way IMO.

> 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;
I still prefer having these flags set by the caller. That way, the
caller has the flexibility to query only the subset needed.

>         *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
>


-- 
-Shyam

Re: [PATCH 2/2] Add SMB 2 support for getting and setting SACLs

[Boris Protopopov]

No objections on my part as far as adding 'if (info & SACL_SECINFO) return'.

I had originally those flags sent from the caller, but that was
confusing at the top level (e.g. in cifsacl.c), so I have opted to
passing only "extra" flags ("additional" was already taken).

-------------
...
> > --- 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);
> Why not have an if (info & SACL_SECINFO) return
> get_smb2_acl_by_path... right at the beginning?
> Looks cleaner that way IMO.
>
...
> > 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;
> I still prefer having these flags set by the caller. That way, the
> caller has the flexibility to query only the subset needed.
> --
> -Shyam

Re: [PATCH 2/2] Add SMB 2 support for getting and setting SACLs

[Steve French]

[-- Attachment #1: Type: text/plain, Size: 6103 bytes --]

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@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@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

[-- Attachment #2: 0001-SMB3-Add-support-for-getting-and-setting-SACLs.patch --]
[-- Type: text/x-patch, Size: 15278 bytes --]

From d42d0c0334ef918310b11818c1805624c0ee9440 Mon Sep 17 00:00:00 2001
From: Boris Protopopov <pboris@amazon.com>
Date: Fri, 18 Dec 2020 11:30:12 -0600
Subject: [PATCH 1/2] SMB3: Add support for getting and setting SACLs

Add SYSTEM_SECURITY access flag and use with smb2 when opening
files for getting/setting SACLs. Add "system.cifs_ntsd_full"
extended attribute to allow user-space access to the functionality.
Avoid multiple server calls when setting owner, DACL, and SACL.

Signed-off-by: Boris Protopopov <pboris@amazon.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/cifsacl.c   | 15 ++++----
 fs/cifs/cifsglob.h  |  4 +--
 fs/cifs/cifspdu.h   |  2 ++
 fs/cifs/cifsproto.h |  4 +--
 fs/cifs/smb2ops.c   | 31 ++++++++++-------
 fs/cifs/smb2pdu.c   |  5 ++-
 fs/cifs/smb2proto.h |  4 +--
 fs/cifs/xattr.c     | 83 +++++++++++++++++++++++++++++++++------------
 8 files changed, 100 insertions(+), 48 deletions(-)

diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
index 2f21f89871cc..2c9051f5e86a 100644
--- a/fs/cifs/cifsacl.c
+++ b/fs/cifs/cifsacl.c
@@ -1195,7 +1195,8 @@ static int build_sec_desc(struct cifs_ntsd *pntsd, struct cifs_ntsd *pnntsd,
 }
 
 struct cifs_ntsd *get_cifs_acl_by_fid(struct cifs_sb_info *cifs_sb,
-		const struct cifs_fid *cifsfid, u32 *pacllen)
+				      const struct cifs_fid *cifsfid, u32 *pacllen,
+				      u32 __maybe_unused unused)
 {
 	struct cifs_ntsd *pntsd = NULL;
 	unsigned int xid;
@@ -1263,7 +1264,7 @@ static struct cifs_ntsd *get_cifs_acl_by_path(struct cifs_sb_info *cifs_sb,
 /* Retrieve an ACL from the server */
 struct cifs_ntsd *get_cifs_acl(struct cifs_sb_info *cifs_sb,
 				      struct inode *inode, const char *path,
-				      u32 *pacllen)
+			       u32 *pacllen, u32 info)
 {
 	struct cifs_ntsd *pntsd = NULL;
 	struct cifsFileInfo *open_file = NULL;
@@ -1273,7 +1274,7 @@ struct cifs_ntsd *get_cifs_acl(struct cifs_sb_info *cifs_sb,
 	if (!open_file)
 		return get_cifs_acl_by_path(cifs_sb, path, pacllen);
 
-	pntsd = get_cifs_acl_by_fid(cifs_sb, &open_file->fid, pacllen);
+	pntsd = get_cifs_acl_by_fid(cifs_sb, &open_file->fid, pacllen, info);
 	cifsFileInfo_put(open_file);
 	return pntsd;
 }
@@ -1338,6 +1339,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 info = 0;
 
 	cifs_dbg(NOISY, "converting ACL to mode for %s\n", path);
 
@@ -1347,9 +1349,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);
+		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);
+		pntsd = ops->get_acl(cifs_sb, inode, path, &acllen, info);
 	else {
 		cifs_put_tlink(tlink);
 		return -EOPNOTSUPP;
@@ -1388,6 +1390,7 @@ id_mode_to_cifs_acl(struct inode *inode, const char *path, __u64 *pnmode,
 	struct tcon_link *tlink = cifs_sb_tlink(cifs_sb);
 	struct smb_version_operations *ops;
 	bool mode_from_sid, id_from_sid;
+	const u32 info = 0;
 
 	if (IS_ERR(tlink))
 		return PTR_ERR(tlink);
@@ -1403,7 +1406,7 @@ id_mode_to_cifs_acl(struct inode *inode, const char *path, __u64 *pnmode,
 		return -EOPNOTSUPP;
 	}
 
-	pntsd = ops->get_acl(cifs_sb, inode, path, &secdesclen);
+	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/cifsglob.h b/fs/cifs/cifsglob.h
index 720d0f6a982d..50fcb65920e8 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -456,9 +456,9 @@ struct smb_version_operations {
 			const char *, const void *, const __u16,
 			const struct nls_table *, struct cifs_sb_info *);
 	struct cifs_ntsd * (*get_acl)(struct cifs_sb_info *, struct inode *,
-			const char *, u32 *);
+			const char *, u32 *, u32);
 	struct cifs_ntsd * (*get_acl_by_fid)(struct cifs_sb_info *,
-			const struct cifs_fid *, u32 *);
+			const struct cifs_fid *, u32 *, u32);
 	int (*set_acl)(struct cifs_ntsd *, __u32, struct inode *, const char *,
 			int);
 	/* writepages retry size */
diff --git a/fs/cifs/cifspdu.h b/fs/cifs/cifspdu.h
index ce51183ecaf4..64fe5a47b5e8 100644
--- a/fs/cifs/cifspdu.h
+++ b/fs/cifs/cifspdu.h
@@ -240,6 +240,8 @@
 #define SYNCHRONIZE           0x00100000  /* The file handle can waited on to */
 					  /* synchronize with the completion  */
 					  /* of an input/output request       */
+#define SYSTEM_SECURITY       0x01000000  /* The system access control list   */
+					  /* can be read and changed          */
 #define GENERIC_ALL           0x10000000
 #define GENERIC_EXECUTE       0x20000000
 #define GENERIC_WRITE         0x40000000
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index bd1c9b038568..340ff81ee87b 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -218,9 +218,9 @@ extern int cifs_acl_to_fattr(struct cifs_sb_info *cifs_sb,
 extern int id_mode_to_cifs_acl(struct inode *inode, const char *path, __u64 *pnmode,
 					kuid_t uid, kgid_t gid);
 extern struct cifs_ntsd *get_cifs_acl(struct cifs_sb_info *, struct inode *,
-					const char *, u32 *);
+				      const char *, u32 *, u32);
 extern struct cifs_ntsd *get_cifs_acl_by_fid(struct cifs_sb_info *,
-						const struct cifs_fid *, u32 *);
+				const struct cifs_fid *, u32 *, u32);
 extern int set_cifs_acl(struct cifs_ntsd *, __u32, struct inode *,
 				const char *, int);
 extern unsigned int setup_authusers_ACE(struct cifs_ace *pace);
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 949cd1177147..4a1761139e00 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -3214,7 +3214,7 @@ smb2_query_reparse_tag(const unsigned int xid, struct cifs_tcon *tcon,
 
 static struct cifs_ntsd *
 get_smb2_acl_by_fid(struct cifs_sb_info *cifs_sb,
-		const struct cifs_fid *cifsfid, u32 *pacllen)
+		    const struct cifs_fid *cifsfid, u32 *pacllen, u32 info)
 {
 	struct cifs_ntsd *pntsd = NULL;
 	unsigned int xid;
@@ -3228,7 +3228,8 @@ get_smb2_acl_by_fid(struct cifs_sb_info *cifs_sb,
 	cifs_dbg(FYI, "trying to get acl\n");
 
 	rc = SMB2_query_acl(xid, tlink_tcon(tlink), cifsfid->persistent_fid,
-			    cifsfid->volatile_fid, (void **)&pntsd, pacllen);
+			    cifsfid->volatile_fid, (void **)&pntsd, pacllen,
+			    info);
 	free_xid(xid);
 
 	cifs_put_tlink(tlink);
@@ -3242,7 +3243,7 @@ get_smb2_acl_by_fid(struct cifs_sb_info *cifs_sb,
 
 static struct cifs_ntsd *
 get_smb2_acl_by_path(struct cifs_sb_info *cifs_sb,
-		const char *path, u32 *pacllen)
+		     const char *path, u32 *pacllen, u32 info)
 {
 	struct cifs_ntsd *pntsd = NULL;
 	u8 oplock = SMB2_OPLOCK_LEVEL_NONE;
@@ -3280,12 +3281,16 @@ get_smb2_acl_by_path(struct cifs_sb_info *cifs_sb,
 	oparms.fid = &fid;
 	oparms.reconnect = false;
 
+	if (info & SACL_SECINFO)
+		oparms.desired_access |= SYSTEM_SECURITY;
+
 	rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, NULL, NULL,
 		       NULL);
 	kfree(utf16_path);
 	if (!rc) {
 		rc = SMB2_query_acl(xid, tlink_tcon(tlink), fid.persistent_fid,
-			    fid.volatile_fid, (void **)&pntsd, pacllen);
+				    fid.volatile_fid, (void **)&pntsd, pacllen,
+				    info);
 		SMB2_close(xid, tcon, fid.persistent_fid, fid.volatile_fid);
 	}
 
@@ -3319,10 +3324,12 @@ set_smb2_acl(struct cifs_ntsd *pnntsd, __u32 acllen,
 	tcon = tlink_tcon(tlink);
 	xid = get_xid();
 
-	if (aclflag == CIFS_ACL_OWNER || aclflag == CIFS_ACL_GROUP)
-		access_flags = WRITE_OWNER;
-	else
-		access_flags = WRITE_DAC;
+	if (aclflag & CIFS_ACL_OWNER || aclflag & CIFS_ACL_GROUP)
+		access_flags |= WRITE_OWNER;
+	if (aclflag & CIFS_ACL_SACL)
+		access_flags |= SYSTEM_SECURITY;
+	if (aclflag & CIFS_ACL_DACL)
+		access_flags |= WRITE_DAC;
 
 	utf16_path = cifs_convert_path_to_utf16(path, cifs_sb);
 	if (!utf16_path) {
@@ -3356,8 +3363,8 @@ set_smb2_acl(struct cifs_ntsd *pnntsd, __u32 acllen,
 /* Retrieve an ACL from the server */
 static struct cifs_ntsd *
 get_smb2_acl(struct cifs_sb_info *cifs_sb,
-				      struct inode *inode, const char *path,
-				      u32 *pacllen)
+	     struct inode *inode, const char *path,
+	     u32 *pacllen, u32 info)
 {
 	struct cifs_ntsd *pntsd = NULL;
 	struct cifsFileInfo *open_file = NULL;
@@ -3365,9 +3372,9 @@ get_smb2_acl(struct cifs_sb_info *cifs_sb,
 	if (inode)
 		open_file = find_readable_file(CIFS_I(inode), true);
 	if (!open_file)
-		return get_smb2_acl_by_path(cifs_sb, path, pacllen);
+		return get_smb2_acl_by_path(cifs_sb, path, pacllen, info);
 
-	pntsd = get_smb2_acl_by_fid(cifs_sb, &open_file->fid, pacllen);
+	pntsd = get_smb2_acl_by_fid(cifs_sb, &open_file->fid, pacllen, info);
 	cifsFileInfo_put(open_file);
 	return pntsd;
 }
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index fc06c762fbbf..202d8742d149 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -3479,10 +3479,9 @@ 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)
+	       u64 persistent_fid, u64 volatile_fid,
+	       void **data, u32 *plen, u32 additional_info)
 {
-	__u32 additional_info = OWNER_SECINFO | GROUP_SECINFO | DACL_SECINFO;
 	*plen = 0;
 
 	return query_info(xid, tcon, persistent_fid, volatile_fid,
diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
index d4110447ee3a..9565e27681a5 100644
--- a/fs/cifs/smb2proto.h
+++ b/fs/cifs/smb2proto.h
@@ -200,8 +200,8 @@ extern int SMB2_query_info_init(struct cifs_tcon *tcon,
 				size_t input_len, void *input);
 extern void SMB2_query_info_free(struct smb_rqst *rqst);
 extern int SMB2_query_acl(const unsigned int xid, struct cifs_tcon *tcon,
-			   u64 persistent_file_id, u64 volatile_file_id,
-			   void **data, unsigned int *plen);
+			  u64 persistent_file_id, u64 volatile_file_id,
+			  void **data, unsigned int *plen, u32 info);
 extern int SMB2_get_srv_num(const unsigned int xid, struct cifs_tcon *tcon,
 			    u64 persistent_fid, u64 volatile_fid,
 			    __le64 *uniqueid);
diff --git a/fs/cifs/xattr.c b/fs/cifs/xattr.c
index b8299173ea7e..9318a2acf4ee 100644
--- a/fs/cifs/xattr.c
+++ b/fs/cifs/xattr.c
@@ -34,6 +34,7 @@
 #define MAX_EA_VALUE_SIZE CIFSMaxBufSize
 #define CIFS_XATTR_CIFS_ACL "system.cifs_acl" /* DACL only */
 #define CIFS_XATTR_CIFS_NTSD "system.cifs_ntsd" /* owner plus DACL */
+#define CIFS_XATTR_CIFS_NTSD_FULL "system.cifs_ntsd_full" /* owner/DACL/SACL */
 #define CIFS_XATTR_ATTRIB "cifs.dosattrib"  /* full name: user.cifs.dosattrib */
 #define CIFS_XATTR_CREATETIME "cifs.creationtime"  /* user.cifs.creationtime */
 /*
@@ -43,12 +44,13 @@
  */
 #define SMB3_XATTR_CIFS_ACL "system.smb3_acl" /* DACL only */
 #define SMB3_XATTR_CIFS_NTSD "system.smb3_ntsd" /* owner plus DACL */
+#define SMB3_XATTR_CIFS_NTSD_FULL "system.smb3_ntsd_full" /* owner/DACL/SACL */
 #define SMB3_XATTR_ATTRIB "smb3.dosattrib"  /* full name: user.smb3.dosattrib */
 #define SMB3_XATTR_CREATETIME "smb3.creationtime"  /* user.smb3.creationtime */
 /* BB need to add server (Samba e.g) support for security and trusted prefix */
 
 enum { XATTR_USER, XATTR_CIFS_ACL, XATTR_ACL_ACCESS, XATTR_ACL_DEFAULT,
-	XATTR_CIFS_NTSD };
+	XATTR_CIFS_NTSD, XATTR_CIFS_NTSD_FULL };
 
 static int cifs_attrib_set(unsigned int xid, struct cifs_tcon *pTcon,
 			   struct inode *inode, char *full_path,
@@ -164,7 +166,8 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
 		break;
 
 	case XATTR_CIFS_ACL:
-	case XATTR_CIFS_NTSD: {
+	case XATTR_CIFS_NTSD:
+	case XATTR_CIFS_NTSD_FULL: {
 		struct cifs_ntsd *pacl;
 
 		if (!value)
@@ -174,23 +177,27 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
 			rc = -ENOMEM;
 		} else {
 			memcpy(pacl, value, size);
-			if (value &&
-			    pTcon->ses->server->ops->set_acl) {
+			if (pTcon->ses->server->ops->set_acl) {
+				int aclflags = 0;
 				rc = 0;
-				if (handler->flags == XATTR_CIFS_NTSD) {
-					/* set owner and DACL */
-					rc = pTcon->ses->server->ops->set_acl(
-							pacl, size, inode,
-							full_path,
-							CIFS_ACL_OWNER);
-				}
-				if (rc == 0) {
-					/* set DACL */
-					rc = pTcon->ses->server->ops->set_acl(
-							pacl, size, inode,
-							full_path,
-							CIFS_ACL_DACL);
+
+				switch (handler->flags) {
+				case XATTR_CIFS_NTSD_FULL:
+					aclflags = (CIFS_ACL_OWNER |
+						    CIFS_ACL_DACL |
+						    CIFS_ACL_SACL);
+					break;
+				case XATTR_CIFS_NTSD:
+					aclflags = (CIFS_ACL_OWNER |
+						    CIFS_ACL_DACL);
+					break;
+				case XATTR_CIFS_ACL:
+				default:
+					aclflags = CIFS_ACL_DACL;
 				}
+
+				rc = pTcon->ses->server->ops->set_acl(pacl,
+					size, inode, full_path, aclflags);
 			} else {
 				rc = -EOPNOTSUPP;
 			}
@@ -327,16 +334,27 @@ static int cifs_xattr_get(const struct xattr_handler *handler,
 		break;
 
 	case XATTR_CIFS_ACL:
-	case XATTR_CIFS_NTSD: {
-		/* the whole ntsd is fetched regardless */
-		u32 acllen;
+	case XATTR_CIFS_NTSD:
+	case XATTR_CIFS_NTSD_FULL: {
+		/*
+		 * fetch owner, DACL, and SACL if asked for full descriptor,
+		 * fetch owner and DACL otherwise
+		 */
+		u32 acllen, additional_info = 0;
 		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;
+		} else {
+			additional_info = OWNER_SECINFO | GROUP_SECINFO |
+				DACL_SECINFO;
+		}
 		pacl = pTcon->ses->server->ops->get_acl(cifs_sb,
-				inode, full_path, &acllen);
+				inode, full_path, &acllen, additional_info);
 		if (IS_ERR(pacl)) {
 			rc = PTR_ERR(pacl);
 			cifs_dbg(VFS, "%s: error %zd getting sec desc\n",
@@ -486,6 +504,27 @@ static const struct xattr_handler smb3_ntsd_xattr_handler = {
 	.set = cifs_xattr_set,
 };
 
+static const struct xattr_handler cifs_cifs_ntsd_full_xattr_handler = {
+	.name = CIFS_XATTR_CIFS_NTSD_FULL,
+	.flags = XATTR_CIFS_NTSD_FULL,
+	.get = cifs_xattr_get,
+	.set = cifs_xattr_set,
+};
+
+/*
+ * Although this is just an alias for the above, need to move away from
+ * confusing users and using the 20 year old term 'cifs' when it is no
+ * longer secure and was replaced by SMB2/SMB3 a long time ago, and
+ * SMB3 and later are highly secure.
+ */
+static const struct xattr_handler smb3_ntsd_full_xattr_handler = {
+	.name = SMB3_XATTR_CIFS_NTSD_FULL,
+	.flags = XATTR_CIFS_NTSD_FULL,
+	.get = cifs_xattr_get,
+	.set = cifs_xattr_set,
+};
+
+
 static const struct xattr_handler cifs_posix_acl_access_xattr_handler = {
 	.name = XATTR_NAME_POSIX_ACL_ACCESS,
 	.flags = XATTR_ACL_ACCESS,
@@ -507,6 +546,8 @@ const struct xattr_handler *cifs_xattr_handlers[] = {
 	&smb3_acl_xattr_handler, /* alias for above since avoiding "cifs" */
 	&cifs_cifs_ntsd_xattr_handler,
 	&smb3_ntsd_xattr_handler, /* alias for above since avoiding "cifs" */
+	&cifs_cifs_ntsd_full_xattr_handler,
+	&smb3_ntsd_full_xattr_handler, /* alias for above since avoiding "cifs" */
 	&cifs_posix_acl_access_xattr_handler,
 	&cifs_posix_acl_default_xattr_handler,
 	NULL
-- 
2.27.0


[-- Attachment #3: 0002-Add-SMB-2-support-for-getting-and-setting-SACLs.patch --]
[-- Type: text/x-patch, Size: 2967 bytes --]

From e1e3e35ae584c441606ae1c305dd377a84b7b51f Mon Sep 17 00:00:00 2001
From: Boris Protopopov <pboris@amazon.com>
Date: Thu, 17 Dec 2020 20:58:08 +0000
Subject: [PATCH 2/2] Add SMB 2 support for getting and setting SACLs

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@amazon.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 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/smb2ops.c b/fs/cifs/smb2ops.c
index 4a1761139e00..f19274857292 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -3369,9 +3369,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 202d8742d149..067eb44c7baa 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -3480,8 +3480,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.27.0