diff mbox series

cifs: fix crash in smb2_compound_op()/smb2_set_next_command()

Message ID 20190716004146.13668-1-lsahlber@redhat.com (mailing list archive)
State New, archived
Headers show
Series cifs: fix crash in smb2_compound_op()/smb2_set_next_command() | expand

Commit Message

Ronnie Sahlberg July 16, 2019, 12:41 a.m. UTC
RHBZ: 1722704

In low memory situations the various SMB2_*_init() functions can fail
to allocate a request PDU and thus leave the request iovector as NULL.

If we don't check the return code for failure we end up calling
smb2_set_next_command() with a NULL iovector causing a crash when it tries
to dereference it.

CC: Stable <stable@vger.kernel.org>
Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/smb2inode.c | 12 ++++++++++++
 fs/cifs/smb2ops.c   | 11 ++++++++++-
 2 files changed, 22 insertions(+), 1 deletion(-)

Comments

Ronnie Sahlberg July 17, 2019, 3:07 a.m. UTC | #1
----- Original Message -----
> From: "Sasha Levin" <sashal@kernel.org>
> To: "Sasha Levin" <sashal@kernel.org>, "Ronnie Sahlberg" <lsahlber@redhat.com>, "linux-cifs"
> <linux-cifs@vger.kernel.org>
> Cc: "Steve French" <smfrench@gmail.com>, "Stable" <stable@vger.kernel.org>, stable@vger.kernel.org
> Sent: Tuesday, 16 July, 2019 11:27:10 AM
> Subject: Re: [PATCH] cifs: fix crash in smb2_compound_op()/smb2_set_next_command()
> 
> Hi,
> 
> [This is an automated email]
> 
> This commit has been processed because it contains a -stable tag.
> The stable tag indicates that it's relevant for the following trees: all
> 
> The bot has tested the following trees: v5.2.1, v5.1.18, v4.19.59, v4.14.133,
> v4.9.185, v4.4.185.
> 
> v5.2.1: Build OK!
> v5.1.18: Build OK!
> v4.19.59: Failed to apply! Possible dependencies:
>     271b9c0c8007 ("smb3: Fix rmdir compounding regression to strict servers")
>     c2e0fe3f5aae ("cifs: make rmdir() use compounding")
>     c5a5f38f075c ("cifs: add a smb2_compound_op and change QUERY_INFO to use
>     it")
>     dcbf91035709 ("cifs: change SMB2_OP_SET_INFO to use compounding")
>     e77fe73c7e38 ("cifs: we can not use small padding iovs together with
>     encryption")
>     f5b05d622a3e ("cifs: add IOCTL for QUERY_INFO passthrough to userspace")
>     f733e3936da4 ("cifs: change mkdir to use a compound")
>     f7bfe04bf0db ("cifs: change SMB2_OP_SET_EOF to use compounding")
> 
> v4.14.133: Failed to apply! Possible dependencies:
>     2e96467d9eb1 ("cifs: add pdu_size to the TCP_Server_Info structure")
>     3d4ef9a15343 ("smb3: fix redundant opens on root")
>     730928c8f4be ("cifs: update smb2_queryfs() to use compounding")
>     74dcf418fe34 ("CIFS: SMBD: Read correct returned data length for RDMA
>     write (SMB read) I/O")
>     8ce79ec359ad ("cifs: update multiplex loop to handle compounded
>     responses")
>     91cb74f5142c ("cifs: Change SMB2_open to return an iov for the error
>     parameter")
>     93012bf98416 ("cifs: add server->vals->header_preamble_size")
>     9d874c36552a ("cifs: fix a buffer leak in smb2_query_symlink")
>     c5a5f38f075c ("cifs: add a smb2_compound_op and change QUERY_INFO to use
>     it")
>     f5b05d622a3e ("cifs: add IOCTL for QUERY_INFO passthrough to userspace")
> 
> v4.9.185: Failed to apply! Possible dependencies:
>     31473fc4f965 ("CIFS: Separate SMB2 header structure")
>     7fb8986e7449 ("CIFS: Add capability to transform requests before
>     sending")
>     8ce79ec359ad ("cifs: update multiplex loop to handle compounded
>     responses")
>     9bb17e0916a0 ("CIFS: Add transform header handling callbacks")
>     b8f57ee8aad4 ("CIFS: Separate RFC1001 length processing for SMB2 read")
>     da502f7df03d ("CIFS: Make SendReceive2() takes resp iov")
>     ef65aaede23f ("smb2: Enforce sec= mount option")
>     f5b05d622a3e ("cifs: add IOCTL for QUERY_INFO passthrough to userspace")
> 
> v4.4.185: Failed to apply! Possible dependencies:
>     141891f4727c ("SMB3: Add mount parameter to allow user to override max
>     credits")
>     166cea4dc3a4 ("SMB2: Separate RawNTLMSSP authentication from
>     SMB2_sess_setup")
>     16c568efff82 ("cifs: merge the hash calculation helpers")
>     275516cdcfa4 ("Print IP address of unresponsive server")
>     31473fc4f965 ("CIFS: Separate SMB2 header structure")
>     373512ec5c10 ("Prepare for encryption support (first part). Add
>     decryption and encryption key generation. Thanks to Metze for helping
>     with this.")
>     3baf1a7b9215 ("SMB2: Separate Kerberos authentication from
>     SMB2_sess_setup")
>     7fb8986e7449 ("CIFS: Add capability to transform requests before
>     sending")
>     834170c85978 ("Enable previous version support")
>     8ce79ec359ad ("cifs: update multiplex loop to handle compounded
>     responses")
>     9bb17e0916a0 ("CIFS: Add transform header handling callbacks")
>     adfeb3e00e8e ("cifs: Make echo interval tunable")
>     da502f7df03d ("CIFS: Make SendReceive2() takes resp iov")
>     ef65aaede23f ("smb2: Enforce sec= mount option")
>     f5b05d622a3e ("cifs: add IOCTL for QUERY_INFO passthrough to userspace")
> 
> 
> NOTE: The patch will not be queued to stable trees until it is upstream.
> 
> How should we proceed with this patch?

So it applies cleanly in  v5.2.1 and v5.1.18.
I think it is sufficient to get into those two versions then.  It is very hard to trigger this issue.


> --
> Thanks,
> Sasha
>
diff mbox series

Patch

diff --git a/fs/cifs/smb2inode.c b/fs/cifs/smb2inode.c
index 278405d26c47..d8d9cdfa30b6 100644
--- a/fs/cifs/smb2inode.c
+++ b/fs/cifs/smb2inode.c
@@ -120,6 +120,8 @@  smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
 				SMB2_O_INFO_FILE, 0,
 				sizeof(struct smb2_file_all_info) +
 					  PATH_MAX * 2, 0, NULL);
+		if (rc)
+			goto finished;
 		smb2_set_next_command(tcon, &rqst[num_rqst]);
 		smb2_set_related(&rqst[num_rqst++]);
 		trace_smb3_query_info_compound_enter(xid, ses->Suid, tcon->tid,
@@ -147,6 +149,8 @@  smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
 					COMPOUND_FID, current->tgid,
 					FILE_DISPOSITION_INFORMATION,
 					SMB2_O_INFO_FILE, 0, data, size);
+		if (rc)
+			goto finished;
 		smb2_set_next_command(tcon, &rqst[num_rqst]);
 		smb2_set_related(&rqst[num_rqst++]);
 		trace_smb3_rmdir_enter(xid, ses->Suid, tcon->tid, full_path);
@@ -163,6 +167,8 @@  smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
 					COMPOUND_FID, current->tgid,
 					FILE_END_OF_FILE_INFORMATION,
 					SMB2_O_INFO_FILE, 0, data, size);
+		if (rc)
+			goto finished;
 		smb2_set_next_command(tcon, &rqst[num_rqst]);
 		smb2_set_related(&rqst[num_rqst++]);
 		trace_smb3_set_eof_enter(xid, ses->Suid, tcon->tid, full_path);
@@ -180,6 +186,8 @@  smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
 					COMPOUND_FID, current->tgid,
 					FILE_BASIC_INFORMATION,
 					SMB2_O_INFO_FILE, 0, data, size);
+		if (rc)
+			goto finished;
 		smb2_set_next_command(tcon, &rqst[num_rqst]);
 		smb2_set_related(&rqst[num_rqst++]);
 		trace_smb3_set_info_compound_enter(xid, ses->Suid, tcon->tid,
@@ -206,6 +214,8 @@  smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
 					COMPOUND_FID, current->tgid,
 					FILE_RENAME_INFORMATION,
 					SMB2_O_INFO_FILE, 0, data, size);
+		if (rc)
+			goto finished;
 		smb2_set_next_command(tcon, &rqst[num_rqst]);
 		smb2_set_related(&rqst[num_rqst++]);
 		trace_smb3_rename_enter(xid, ses->Suid, tcon->tid, full_path);
@@ -231,6 +241,8 @@  smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
 					COMPOUND_FID, current->tgid,
 					FILE_LINK_INFORMATION,
 					SMB2_O_INFO_FILE, 0, data, size);
+		if (rc)
+			goto finished;
 		smb2_set_next_command(tcon, &rqst[num_rqst]);
 		smb2_set_related(&rqst[num_rqst++]);
 		trace_smb3_hardlink_enter(xid, ses->Suid, tcon->tid, full_path);
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 4b0b14946343..462890f4cb9c 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -2027,6 +2027,10 @@  smb2_set_related(struct smb_rqst *rqst)
 	struct smb2_sync_hdr *shdr;
 
 	shdr = (struct smb2_sync_hdr *)(rqst->rq_iov[0].iov_base);
+	if (shdr == NULL) {
+		cifs_dbg(FYI, "shdr NULL in smb2_set_related\n");
+		return;
+	}
 	shdr->Flags |= SMB2_FLAGS_RELATED_OPERATIONS;
 }
 
@@ -2041,6 +2045,12 @@  smb2_set_next_command(struct cifs_tcon *tcon, struct smb_rqst *rqst)
 	unsigned long len = smb_rqst_len(server, rqst);
 	int i, num_padding;
 
+	shdr = (struct smb2_sync_hdr *)(rqst->rq_iov[0].iov_base);
+	if (shdr == NULL) {
+		cifs_dbg(FYI, "shdr NULL in smb2_set_next_command\n");
+		return;
+	}
+
 	/* SMB headers in a compound are 8 byte aligned. */
 
 	/* No padding needed */
@@ -2080,7 +2090,6 @@  smb2_set_next_command(struct cifs_tcon *tcon, struct smb_rqst *rqst)
 	}
 
  finished:
-	shdr = (struct smb2_sync_hdr *)(rqst->rq_iov[0].iov_base);
 	shdr->NextCommand = cpu_to_le32(len);
 }