cifs: various fixes for NULL pointers

PATCH New Ronnie Sahlberg 1 participant 13 messages Oct 11, 2017 1 message not in the archive

Subsystems (2)

SMB client (CIFS) Kernel SMB3 server (ksmbd)

lore

Share & tools

lore

Ronnie Sahlberg

author

2017-10-11 01:59 UTC #0 lore raw

cover cifs: various fixes for NULL pointers

List, Steve

We recently had two very similar issues of dereferencing uninitialized pointer when Sendreceive2() would have certain failures.

I went through the rest of smb2pdu.c to check for similar issues.

These are what I found. Including a memory leak in smb2_lockv().

These patches are all small and kept separate for review.
It might make sense to collapse all these patches into a single one. I don't know. Up to you.

A message that is not in the archive (sent off-list or not yet received): 20171011015927.7669-1-lsahlber-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org

Ronnie Sahlberg

author

2017-10-11 01:59 UTC #2 lore raw

PATCH 1/12 cifs: initialize rsp_iov in SMB2_negotiate

Initialize rsp_iov so that when rsp is assigned we know it is either NULL or a valid pointer.

Otherwise, if SendReceive2() returns an error without setting rsp_iov we could end up calling free_rsp_buf() on an uninitialized pointer.

Signed-off-by Ronnie Sahlberg <[email protected]>

---
 fs/cifs/smb2pdu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

fs/cifs/smb2pdu.c +1 -1

index 6ff4c275ca9a..08701b6125b9 100644
@@ -470,7 +470,7 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
 	struct smb2_negotiate_req *req;
 	struct smb2_negotiate_rsp *rsp;
 	struct kvec iov[1];
-	struct kvec rsp_iov;
+	struct kvec rsp_iov = { NULL, 0 };
 	int rc = 0;
 	int resp_buftype;
 	struct TCP_Server_Info *server = ses->server;

Signature · 1 line

2.13.3

Ronnie Sahlberg

author

2017-10-11 01:59 UTC #3 lore raw

PATCH 2/12 cifs: initialize rsp_iov in SMB2_ioctl and check for NULL before deref

In reply to a message not in the archive

Signed-off-by Ronnie Sahlberg <[email protected]>

---
 fs/cifs/smb2pdu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

fs/cifs/smb2pdu.c +3 -1

index 08701b6125b9..bf35755501e3 100644
@@ -1884,7 +1884,7 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
 	struct smb2_sync_hdr *shdr;
 	struct cifs_ses *ses;
 	struct kvec iov[2];
-	struct kvec rsp_iov;
+	struct kvec rsp_iov = { NULL, 0 };
 	int resp_buftype;
 	int n_iov;
 	int rc = 0;

@@ -1981,6 +1981,8 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
 	rc = SendReceive2(xid, ses, iov, n_iov, &resp_buftype, flags, &rsp_iov);
 	cifs_small_buf_release(req);
 	rsp = (struct smb2_ioctl_rsp *)rsp_iov.iov_base;
+	if (rsp == NULL)
+		goto ioctl_exit;
 
 	if ((rc != 0) && (rc != -EINVAL)) {
 		cifs_stats_fail_inc(tcon, SMB2_IOCTL_HE);

Signature · 1 line

2.13.3

Ronnie Sahlberg

author

2017-10-11 01:59 UTC #4 lore raw

PATCH 3/12 cifs: initialize rsp_iov in SMB2_close

In reply to a message not in the archive

Or else we might call free_rsp_buf() on an uninitialized pointer if SendReceive2() failed.

Signed-off-by Ronnie Sahlberg <[email protected]>

---
 fs/cifs/smb2pdu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

fs/cifs/smb2pdu.c +1 -1

index bf35755501e3..acadbdbfc8cb 100644
@@ -2066,7 +2066,7 @@ SMB2_close(const unsigned int xid, struct cifs_tcon *tcon,
 	struct smb2_close_rsp *rsp;
 	struct cifs_ses *ses = tcon->ses;
 	struct kvec iov[1];
-	struct kvec rsp_iov;
+	struct kvec rsp_iov = { NULL, 0 };
 	int resp_buftype;
 	int rc = 0;
 	int flags = 0;

Signature · 1 line

2.13.3

Ronnie Sahlberg

author

2017-10-11 01:59 UTC #5 lore raw

PATCH 4/12 cifs: initialize rsp_iov in SMB2_query_info

In reply to a message not in the archive

Or else we might call free_rsp_buf() on an uninitialized pointer if SendReceive2() failed

Signed-off-by Ronnie Sahlberg <[email protected]>

---
 fs/cifs/smb2pdu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

fs/cifs/smb2pdu.c +2 -2

index acadbdbfc8cb..97dc913b8dd7 100644
@@ -2170,9 +2170,9 @@ query_info(const unsigned int xid, struct cifs_tcon *tcon,
 		u32 *dlen)
 {
 	struct smb2_query_info_req *req;
-	struct smb2_query_info_rsp *rsp = NULL;
+	struct smb2_query_info_rsp *rsp;
 	struct kvec iov[2];
-	struct kvec rsp_iov;
+	struct kvec rsp_iov = { NULL, 0 };
 	int rc = 0;
 	int resp_buftype;
 	struct cifs_ses *ses = tcon->ses;

Signature · 1 line

2.13.3

Ronnie Sahlberg

author

2017-10-11 01:59 UTC #6 lore raw

PATCH 5/12 cifs: initialize rsp_iov in SMB2_flush

In reply to a message not in the archive

Or else we might call free_rsp_buf() on an uninitialized pointer if SendReceive2() failed

Signed-off-by Ronnie Sahlberg <[email protected]>

---
 fs/cifs/smb2pdu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

fs/cifs/smb2pdu.c +1 -1

index 97dc913b8dd7..ca861ee4ac0a 100644
@@ -2406,7 +2406,7 @@ SMB2_flush(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
 	struct smb2_flush_req *req;
 	struct cifs_ses *ses = tcon->ses;
 	struct kvec iov[1];
-	struct kvec rsp_iov;
+	struct kvec rsp_iov = { NULL, 0 };
 	int resp_buftype;
 	int rc = 0;
 	int flags = 0;

Signature · 1 line

2.13.3

Ronnie Sahlberg

author

2017-10-11 01:59 UTC #7 lore raw

PATCH 6/12 cifs: initialize rsp_iov in SMB2_read and add check for NULL

In reply to a message not in the archive

Signed-off-by Ronnie Sahlberg <[email protected]>

---
 fs/cifs/smb2pdu.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

fs/cifs/smb2pdu.c +6 -2

index ca861ee4ac0a..96a6ed9c86d4 100644
@@ -2641,10 +2641,10 @@ SMB2_read(const unsigned int xid, struct cifs_io_parms *io_parms,
 {
 	int resp_buftype, rc = -EACCES;
 	struct smb2_read_plain_req *req = NULL;
-	struct smb2_read_rsp *rsp = NULL;
+	struct smb2_read_rsp *rsp;
 	struct smb2_sync_hdr *shdr;
 	struct kvec iov[2];
-	struct kvec rsp_iov;
+	struct kvec rsp_iov = { NULL, 0 };
 	unsigned int total_len;
 	__be32 req_len;
 	struct smb_rqst rqst = { .rq_iov = iov,

@@ -2671,6 +2671,10 @@ SMB2_read(const unsigned int xid, struct cifs_io_parms *io_parms,
 	cifs_small_buf_release(req);
 
 	rsp = (struct smb2_read_rsp *)rsp_iov.iov_base;
+	if (rsp == NULL) {
+		cifs_dbg(VFS, "rsp is NULL in read\n");
+		return -EIO;
+	}
 	shdr = get_sync_hdr(rsp);
 
 	if (shdr->Status == STATUS_END_OF_FILE) {

Signature · 1 line

2.13.3

Ronnie Sahlberg

author

2017-10-11 01:59 UTC #8 lore raw

PATCH 7/12 cifs: initialize rsp_iov in SMB2_write

In reply to a message not in the archive

Or else we might call free_rsp_buf() on an uninitialized pointer if SendReceive2() failed

Signed-off-by Ronnie Sahlberg <[email protected]>

---
 fs/cifs/smb2pdu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

fs/cifs/smb2pdu.c +2 -2

index 96a6ed9c86d4..21ffeb5f5e4b 100644
@@ -2862,9 +2862,9 @@ SMB2_write(const unsigned int xid, struct cifs_io_parms *io_parms,
 {
 	int rc = 0;
 	struct smb2_write_req *req = NULL;
-	struct smb2_write_rsp *rsp = NULL;
+	struct smb2_write_rsp *rsp;
 	int resp_buftype;
-	struct kvec rsp_iov;
+	struct kvec rsp_iov = { NULL, 0 };
 	int flags = 0;
 
 	*nbytes = 0;

Signature · 1 line

2.13.3

Ronnie Sahlberg

author

2017-10-11 01:59 UTC #9 lore raw

PATCH 8/12 cifs: initialize rsp_iov in SMB2_query_directory

In reply to a message not in the archive

Or else we might call free_rsp_buf() on an uninitialized pointer if SendReceive2() failed

Signed-off-by Ronnie Sahlberg <[email protected]>

---
 fs/cifs/smb2pdu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

fs/cifs/smb2pdu.c +1 -1

index 21ffeb5f5e4b..b7f59f536fd7 100644
@@ -2969,7 +2969,7 @@ SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
 	struct smb2_query_directory_req *req;
 	struct smb2_query_directory_rsp *rsp = NULL;
 	struct kvec iov[2];
-	struct kvec rsp_iov;
+	struct kvec rsp_iov = { NULL, 0 };
 	int rc = 0;
 	int len;
 	int resp_buftype = CIFS_NO_BUFFER;

Signature · 1 line

2.13.3

Ronnie Sahlberg

author

2017-10-11 01:59 UTC #10 lore raw

PATCH 9/12 cifs: initialize rsp_iov in send_set_info

In reply to a message not in the archive

Or else we might call free_rsp_buf() on an uninitialized pointer if SendReceive2() failed

Signed-off-by Ronnie Sahlberg <[email protected]>

---
 fs/cifs/smb2pdu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

fs/cifs/smb2pdu.c +2 -2

index b7f59f536fd7..93ecaa050c57 100644
@@ -3099,9 +3099,9 @@ send_set_info(const unsigned int xid, struct cifs_tcon *tcon,
 		void **data, unsigned int *size)
 {
 	struct smb2_set_info_req *req;
-	struct smb2_set_info_rsp *rsp = NULL;
+	struct smb2_set_info_rsp *rsp;
 	struct kvec *iov;
-	struct kvec rsp_iov;
+	struct kvec rsp_iov = { NULL, 0 };
 	int rc = 0;
 	int resp_buftype;
 	unsigned int i;

Signature · 1 line

2.13.3

Ronnie Sahlberg

author

2017-10-11 01:59 UTC #11 lore raw

PATCH 10/12 cifs: initialize rsp_iov in SMB2_QFS_info

In reply to a message not in the archive

Or else we might call free_rsp_buf() on an uninitialized pointer if SendReceive2() failed

Signed-off-by Ronnie Sahlberg <[email protected]>

---
 fs/cifs/smb2pdu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

fs/cifs/smb2pdu.c +2 -2

index 93ecaa050c57..f3cc9dbd6fd4 100644
@@ -3382,9 +3382,9 @@ int
 SMB2_QFS_info(const unsigned int xid, struct cifs_tcon *tcon,
 	      u64 persistent_fid, u64 volatile_fid, struct kstatfs *fsdata)
 {
-	struct smb2_query_info_rsp *rsp = NULL;
+	struct smb2_query_info_rsp *rsp;
 	struct kvec iov;
-	struct kvec rsp_iov;
+	struct kvec rsp_iov = { NULL, 0 };
 	int rc = 0;
 	int resp_buftype;
 	struct cifs_ses *ses = tcon->ses;

Signature · 1 line

2.13.3

Ronnie Sahlberg

author

2017-10-11 01:59 UTC #12 lore raw

PATCH 11/12 cifs: initialize rsp_iov in SMB2_QFS_attr

In reply to a message not in the archive

Or else we might call free_rsp_buf() on an uninitialized pointer if SendReceive2() failed

Signed-off-by Ronnie Sahlberg <[email protected]>

---
 fs/cifs/smb2pdu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

fs/cifs/smb2pdu.c +2 -2

index f3cc9dbd6fd4..9dc350fc283d 100644
@@ -3425,9 +3425,9 @@ int
 SMB2_QFS_attr(const unsigned int xid, struct cifs_tcon *tcon,
 	      u64 persistent_fid, u64 volatile_fid, int level)
 {
-	struct smb2_query_info_rsp *rsp = NULL;
+	struct smb2_query_info_rsp *rsp;
 	struct kvec iov;
-	struct kvec rsp_iov;
+	struct kvec rsp_iov = { NULL, 0 };
 	int rc = 0;
 	int resp_buftype, max_len, min_len;
 	struct cifs_ses *ses = tcon->ses;

Signature · 1 line

2.13.3

Ronnie Sahlberg

author

2017-10-11 01:59 UTC #13 lore raw

PATCH 12/12 cifs: initialize rsp_iov in smb2_lockv and fix a memory leak

In reply to a message not in the archive

Signed-off-by Ronnie Sahlberg <[email protected]>

---
 fs/cifs/smb2pdu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

fs/cifs/smb2pdu.c +2 -1

index 9dc350fc283d..fd2b2b8e86b0 100644
@@ -3498,7 +3498,7 @@ smb2_lockv(const unsigned int xid, struct cifs_tcon *tcon,
 	int rc = 0;
 	struct smb2_lock_req *req = NULL;
 	struct kvec iov[2];
-	struct kvec rsp_iov;
+	struct kvec rsp_iov = { NULL, 0 };
 	int resp_buf_type;
 	unsigned int count;
 	int flags = CIFS_NO_RESP;

@@ -3531,6 +3531,7 @@ smb2_lockv(const unsigned int xid, struct cifs_tcon *tcon,
 	rc = SendReceive2(xid, tcon->ses, iov, 2, &resp_buf_type, flags,
 			  &rsp_iov);
 	cifs_small_buf_release(req);
+	free_rsp_buf(resp_buf_type, rsp_iov.iov_base);
 	if (rc) {
 		cifs_dbg(FYI, "Send error in smb2_lockv = %d\n", rc);
 		cifs_stats_fail_inc(tcon, SMB2_LOCK_HE);

Signature · 1 line

2.13.3