[PATCH] CIFS: fix FSCTL_SET_REPARSE_POINT SMB2_ioctl() call

[PATCH] CIFS: fix FSCTL_SET_REPARSE_POINT SMB2_ioctl() call

[Steve French]

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

merged Aurelien's small patch (attached) into cifs-2.6.git for-next

-- 
Thanks,

Steve

[-- Attachment #2: 0001-CIFS-fix-FSCTL_SET_REPARSE_POINT-SMB2_ioctl-call.patch --]
[-- Type: text/x-patch, Size: 2273 bytes --]

From 778d81b65e4d596251943002522d94a7c6fbcf69 Mon Sep 17 00:00:00 2001
From: Aurelien Aptel <aaptel@suse.com>
Date: Mon, 4 Mar 2019 18:50:18 +0100
Subject: [PATCH] CIFS: fix FSCTL_SET_REPARSE_POINT SMB2_ioctl() call

Without this change the ioctl() fails with INVALID_PARAMETER.
Since SET_REPARSE_POINT has no output, set the max output response
size to zero.

[MS-SMB2] reads 3.3.5.15 Receiving an SMB2 IOCTL Request

If either InputCount, MaxInputResponse, or MaxOutputResponse is
greater than Connection.MaxTransactSize, the server SHOULD<306> fail
the request with STATUS_INVALID_PARAMETER.

The server MUST fail the request with STATUS_INVALID_PARAMETER in the following cases:

* If InputOffset is greater than zero but less than (size of SMB2
  header + size of the SMB2 IOCTL request not including Buffer) or
  if InputOffset is greater than (size of SMB2 header + size of the
  SMB2 IOCTL request).

* If OutputOffset is greater than zero but less than (size of SMB2
  header + size of the SMB2 IOCTL request not including Buffer) or if
  OutputOffset is greater than (size of SMB2 header + size of the SMB2
  IOCTL request).

* If (InputOffset + InputCount) is greater than (size of SMB2 header +
  size of the SMB2 IOCTL request).

* If (OutputOffset + OutputCount) is greater than (size of SMB2 header
  + size of the SMB2 IOCTL request).

* If OutputCount is greater than zero and OutputOffset is less
  than (InputOffset + InputCount).

Signed-off-by: Aurelien Aptel <aaptel@suse.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/smb2pdu.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 733021566356..cacdf9bf9ef3 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -2539,7 +2539,10 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
 	 * in responses (except for read responses which can be bigger.
 	 * We may want to bump this limit up
 	 */
-	req->MaxOutputResponse = cpu_to_le32(CIFSMaxBufSize);
+	if (opcode == FSCTL_SET_REPARSE_POINT)
+		req->MaxOutputResponse = cpu_to_le32(0);
+	else
+		req->MaxOutputResponse = cpu_to_le32(CIFSMaxBufSize);
 
 	if (is_fsctl)
 		req->Flags = cpu_to_le32(SMB2_0_IOCTL_IS_FSCTL);
-- 
2.17.1

Re: [PATCH] CIFS: fix FSCTL_SET_REPARSE_POINT SMB2_ioctl() call

[ronnie sahlberg]

Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com>

On Tue, Mar 5, 2019 at 4:16 PM Steve French <smfrench@gmail.com> wrote:
>
> merged Aurelien's small patch (attached) into cifs-2.6.git for-next
>
> --
> Thanks,
>
> Steve

Re: [PATCH] CIFS: fix FSCTL_SET_REPARSE_POINT SMB2_ioctl() call

[Aurélien Aptel]

Steve French <smfrench@gmail.com> writes:
> Without this change the ioctl() fails with INVALID_PARAMETER.
> Since SET_REPARSE_POINT has no output, set the max output response
> size to zero.

Note that this is more of a work around, I didn't intend to send this
yet. I haven't tried to figure out which cases we were violating and
where was the underlying issue.

> If either InputCount, MaxInputResponse, or MaxOutputResponse is
> greater than Connection.MaxTransactSize, the server SHOULD<306> fail
> the request with STATUS_INVALID_PARAMETER.

possible

> The server MUST fail the request with STATUS_INVALID_PARAMETER in the following cases:
>
> * If InputOffset is greater than zero but less than (size of SMB2
>   header + size of the SMB2 IOCTL request not including Buffer) or
>   if InputOffset is greater than (size of SMB2 header + size of the
>   SMB2 IOCTL request).

input related, nope

> * If OutputOffset is greater than zero but less than (size of SMB2
>   header + size of the SMB2 IOCTL request not including Buffer) or if
>   OutputOffset is greater than (size of SMB2 header + size of the SMB2
>   IOCTL request).

doesn't involve MaxOutputResponse, nope

> * If (InputOffset + InputCount) is greater than (size of SMB2 header +
>   size of the SMB2 IOCTL request).

input related, nope

> * If (OutputOffset + OutputCount) is greater than (size of SMB2 header
>   + size of the SMB2 IOCTL request).

doesn't involve MaxOutputResponse, nope

> * If OutputCount is greater than zero and OutputOffset is less
>   than (InputOffset + InputCount).

doesn't involve MaxOutputResponse, nope

So it seems to me only possible explanation is the first, i.e.
MaxOutputResponse > Connection.MaxTransactSize, somehow.

-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)

Re: [PATCH] CIFS: fix FSCTL_SET_REPARSE_POINT SMB2_ioctl() call

[Aurélien Aptel]

Aurélien Aptel <aaptel@suse.com> writes:
> So it seems to me only possible explanation is the first, i.e.
> MaxOutputResponse > Connection.MaxTransactSize, somehow.

So I've dumped a few fields:

Connection.MaxTransactSize is set by the server in NEGPROT response.

Against my WS2016, MaxTransactSize = 8388608

But we do this:

	server->maxBuf = min_t(unsigned int, le32_to_cpu(rsp->MaxTransactSize),
			       SMB2_MAX_BUFFER_SIZE);

Since SMB2_MAX_BUFFER_SIZE = 65536, maxBuf is effectively SMB2_MAX_BUFFER_SIZE

And finally, the value used for ioctl MaxOutputResponse is
CIFSMaxBufSize, which is 16384.

I've tried setting MaxOutputResponse to server->maxBuf or
Connection.MaxTransactSize but it always fails the same way... so I
don't know. I guess it must say somewhere that ioctl() with no output
should set MaxOutputResponse to zero?

-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)

Re: [PATCH] CIFS: fix FSCTL_SET_REPARSE_POINT SMB2_ioctl() call

[Aurélien Aptel]

Aurélien Aptel <aaptel@suse.com> writes:
> I've tried setting MaxOutputResponse to server->maxBuf or
> Connection.MaxTransactSize but it always fails the same way... so I
> don't know. I guess it must say somewhere that ioctl() with no output
> should set MaxOutputResponse to zero?

I've just checked what Windows does and it seems to be that as
well. Creating a symlink (mklink link target) makes an
ioctl(SET_REPARSE_POINT) with MaxOutputResponse set to 0.

So I would keep this patch as is.

-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)

RE: [PATCH] CIFS: fix FSCTL_SET_REPARSE_POINT SMB2_ioctl() call

[Tom Talpey]

> -----Original Message-----
> From: linux-cifs-owner@vger.kernel.org <linux-cifs-owner@vger.kernel.org> On
> Behalf Of Aurélien Aptel
> Sent: Tuesday, March 5, 2019 10:41 AM
> To: Steve French <smfrench@gmail.com>; CIFS <linux-cifs@vger.kernel.org>
> Subject: Re: [PATCH] CIFS: fix FSCTL_SET_REPARSE_POINT SMB2_ioctl() call
> 
> Aurélien Aptel <aaptel@suse.com> writes:
> > I've tried setting MaxOutputResponse to server->maxBuf or
> > Connection.MaxTransactSize but it always fails the same way... so I
> > don't know. I guess it must say somewhere that ioctl() with no output
> > should set MaxOutputResponse to zero?
> 
> I've just checked what Windows does and it seems to be that as
> well. Creating a symlink (mklink link target) makes an
> ioctl(SET_REPARSE_POINT) with MaxOutputResponse set to 0.
> 
> So I would keep this patch as is.

Remember that in addition to the SMB server validation, the request goes
to the filesystem, which has its own validation and error status returns. The
STATUS_INVALID_PARAMETER can come back from any layer.

If the MaxOutputResponse != 0 set_reparse_point behavior isn't covered
in either MS-SMB2 or MS-FSA, I'm happy to take a doc bug.

Tom.