diff mbox series

[SMB3]

Message ID CAH2r5muDfiv2=VpUDTkbW3h+05ubMHsMe53nzjwp9apu=-1-tA@mail.gmail.com (mailing list archive)
State New, archived
Headers show
Series [SMB3] | expand

Commit Message

Steve French March 13, 2019, 7:28 a.m. UTC 
SMB3: passthru query info doesn't check for SMB3 FSCTL passthru

The passthrough queries from user space tools like smbinfo can be either
SMB3 QUERY_INFO or SMB3 FSCTL, but we are not checking for the latter.
Temporarily we return EOPNOTSUPP for SMB3 FSCTL passthrough requests
but once compounding fsctls is fixed we can remove the EOPNOTSUPP.

Comments

ronnie sahlberg March 13, 2019, 7:33 a.m. UTC | #1 
Looks good.

Minor nit is
+ } else if (qi.flags != PASSTHRU_QUERY_INFO) {
+ cifs_dbg(VFS, "invalid passthru query flags: 0x%x\n", qi.flags);
+ rc = -EINVAL;
+ } else {
+ memset(&qi_iov, 0, sizeof(qi_iov));
+ rqst[1].rq_iov = qi_iov;
+ rqst[1].rq_nvec = 1;

I think it is clearer if the "nothing matched, error out" conditional
is the last one in the chain.
I.e. swap them around.

reviewedby me

On Wed, Mar 13, 2019 at 5:29 PM Steve French <smfrench@gmail.com> wrote:
>
> SMB3: passthru query info doesn't check for SMB3 FSCTL passthru
>
> The passthrough queries from user space tools like smbinfo can be either
> SMB3 QUERY_INFO or SMB3 FSCTL, but we are not checking for the latter.
> Temporarily we return EOPNOTSUPP for SMB3 FSCTL passthrough requests
> but once compounding fsctls is fixed we can remove the EOPNOTSUPP.
>
>
> --
> Thanks,
>
> Steve
Steve French March 13, 2019, 7:42 a.m. UTC | #2 
Rearranged the clauses per Ronnie's suggestion.  Ver2 attached


On Wed, Mar 13, 2019 at 2:33 AM ronnie sahlberg
<ronniesahlberg@gmail.com> wrote:
>
> Looks good.
>
> Minor nit is
> + } else if (qi.flags != PASSTHRU_QUERY_INFO) {
> + cifs_dbg(VFS, "invalid passthru query flags: 0x%x\n", qi.flags);
> + rc = -EINVAL;
> + } else {
> + memset(&qi_iov, 0, sizeof(qi_iov));
> + rqst[1].rq_iov = qi_iov;
> + rqst[1].rq_nvec = 1;
>
> I think it is clearer if the "nothing matched, error out" conditional
> is the last one in the chain.
> I.e. swap them around.
>
> reviewedby me
>
> On Wed, Mar 13, 2019 at 5:29 PM Steve French <smfrench@gmail.com> wrote:
> >
> > SMB3: passthru query info doesn't check for SMB3 FSCTL passthru
> >
> > The passthrough queries from user space tools like smbinfo can be either
> > SMB3 QUERY_INFO or SMB3 FSCTL, but we are not checking for the latter.
> > Temporarily we return EOPNOTSUPP for SMB3 FSCTL passthrough requests
> > but once compounding fsctls is fixed we can remove the EOPNOTSUPP.
> >
> >
> > --
> > Thanks,
> >
> > Steve
diff mbox series

Patch

From 70d2deccf6ff82e8709061ff998cec25d050cdf5 Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Wed, 13 Mar 2019 02:23:48 -0500
Subject: [PATCH] SMB3: passthru query info doesn't check for SMB3 FSCTL
 passthru

The passthrough queries from user space tools like smbinfo can be either
SMB3 QUERY_INFO or SMB3 FSCTL, but we are not checking for the latter.
Temporarily we return EOPNOTSUPP for SMB3 FSCTL passthrough requests
but once compounding fsctls is fixed can enable.

Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/cifs_ioctl.h |  3 +++
 fs/cifs/smb2ops.c    | 23 +++++++++++++++++------
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/fs/cifs/cifs_ioctl.h b/fs/cifs/cifs_ioctl.h
index d8bce2f862de..086ddc5108af 100644
--- a/fs/cifs/cifs_ioctl.h
+++ b/fs/cifs/cifs_ioctl.h
@@ -43,6 +43,9 @@  struct smb_snapshot_array {
 	/*	snapshots[]; */
 } __packed;
 
+/* query_info flags */
+#define PASSTHRU_QUERY_INFO	0x00000000
+#define PASSTHRU_FSCTL		0x00000001
 struct smb_query_info {
 	__u32   info_type;
 	__u32   file_info_class;
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 823a58550dfd..43b3a9b0096f 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -1390,15 +1390,26 @@  smb2_ioctl_query_info(const unsigned int xid,
 	smb2_set_next_command(tcon, &rqst[0]);
 
 	/* Query */
-	memset(&qi_iov, 0, sizeof(qi_iov));
-	rqst[1].rq_iov = qi_iov;
-	rqst[1].rq_nvec = 1;
+	if (qi.flags & PASSTHRU_FSCTL) {
+		/* Can eventually relax perm check since server enforces too */
+		if (!capable(CAP_SYS_ADMIN))
+			rc = -EPERM;
+		else  /* TBD: Add code to compound FSCTL */
+			rc = -EOPNOTSUPP;
+	} else if (qi.flags != PASSTHRU_QUERY_INFO) {
+		cifs_dbg(VFS, "invalid passthru query flags: 0x%x\n", qi.flags);
+		rc = -EINVAL;
+	} else {
+		memset(&qi_iov, 0, sizeof(qi_iov));
+		rqst[1].rq_iov = qi_iov;
+		rqst[1].rq_nvec = 1;
 
-	rc = SMB2_query_info_init(tcon, &rqst[1], COMPOUND_FID, COMPOUND_FID,
-				  qi.file_info_class, qi.info_type,
-				  qi.additional_information,
+		rc = SMB2_query_info_init(tcon, &rqst[1], COMPOUND_FID,
+				  COMPOUND_FID, qi.file_info_class,
+				  qi.info_type, qi.additional_information,
 				  qi.input_buffer_length,
 				  qi.output_buffer_length, buffer);
+	}
 	if (rc)
 		goto iqinf_exit;
 	smb2_set_next_command(tcon, &rqst[1]);
-- 
2.17.1