Message ID | CAH2r5msqeMuatKpE2Kg6A+935zJ+yeocVHQTipQechbbgNrEXw@mail.gmail.com |
---|---|
State | New |
Headers | show |
Series | CIFS: fix FSCTL_SET_REPARSE_POINT SMB2_ioctl() call | expand |
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
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 <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 <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.
> -----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.
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