lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 12 Jul 2018 13:02:11 +0200
From:   Tomas Bortoli <tomasbortoli@...il.com>
To:     ericvh@...il.com, rminnich@...dia.gov, lucho@...kov.net
Cc:     asmadeus@...ewreck.org, viro@...IV.linux.org.uk,
        davem@...emloft.net, v9fs-developer@...ts.sourceforge.net,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        syzkaller@...glegroups.com, Tomas Bortoli <tomasbortoli@...il.com>
Subject: [V9fs-developer] [PATCH] p9_parse_header() validate PDU length

This patch adds checks to the p9_parse_header() function to
verify that the length found within the header coincides with the actual
length of the PDU. Furthermore, it checks that the length stays within the
acceptable range. To do this the patch brings the actual length of the PDU
from the different transport layers (rdma and virtio). For TCP (trans_fd.c)
the length is not know before, so we get it from the header but we check it
anyway that it's within the valid range.

Signed-off-by: Tomas Bortoli <tomasbortoli@...il.com>
Reported-by: syzbot+65c6b72f284a39d416b4@...kaller.appspotmail.com
---
 net/9p/client.c       | 26 ++++++++++++++++----------
 net/9p/trans_fd.c     | 17 ++++++-----------
 net/9p/trans_rdma.c   |  2 +-
 net/9p/trans_virtio.c |  6 ++++--
 4 files changed, 27 insertions(+), 24 deletions(-)

diff --git a/net/9p/client.c b/net/9p/client.c
index 18c5271910dc..119de44f49e9 100644
--- a/net/9p/client.c
+++ b/net/9p/client.c
@@ -477,20 +477,11 @@ p9_parse_header(struct p9_fcall *pdu, int32_t *size, int8_t *type, int16_t *tag,
 	int err;
 
 	pdu->offset = 0;
-	if (pdu->size == 0)
-		pdu->size = 7;
 
 	err = p9pdu_readf(pdu, 0, "dbw", &r_size, &r_type, &r_tag);
 	if (err)
 		goto rewind_and_exit;
 
-	pdu->size = r_size;
-	pdu->id = r_type;
-	pdu->tag = r_tag;
-
-	p9_debug(P9_DEBUG_9P, "<<< size=%d type: %d tag: %d\n",
-		 pdu->size, pdu->id, pdu->tag);
-
 	if (type)
 		*type = r_type;
 	if (tag)
@@ -498,6 +489,21 @@ p9_parse_header(struct p9_fcall *pdu, int32_t *size, int8_t *type, int16_t *tag,
 	if (size)
 		*size = r_size;
 
+	if (pdu->size != r_size) {
+		err = -EINVAL;
+		goto rewind_and_exit;
+	}
+	if (pdu->size >= pdu->capacity || pdu->size < 7) {
+		p9_debug(P9_DEBUG_ERROR,
+			 "requested packet size too big or too small: %d\n",
+			 pdu->size);
+		return -EIO;
+	}
+	pdu->id = r_type;
+	pdu->tag = r_tag;
+
+	p9_debug(P9_DEBUG_9P, "<<< size=%d type: %d tag: %d\n",
+		 pdu->size, pdu->id, pdu->tag);
 
 rewind_and_exit:
 	if (rewind)
@@ -1575,7 +1581,7 @@ p9_client_read(struct p9_fid *fid, u64 offset, struct iov_iter *to, int *err)
 		int count = iov_iter_count(to);
 		int rsize, non_zc = 0;
 		char *dataptr;
-			
+
 		rsize = fid->iounit;
 		if (!rsize || rsize > clnt->msize-P9_IOHDRSZ)
 			rsize = clnt->msize - P9_IOHDRSZ;
diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
index 588bf88c3305..bf459ee0feab 100644
--- a/net/9p/trans_fd.c
+++ b/net/9p/trans_fd.c
@@ -323,22 +323,16 @@ static void p9_read_work(struct work_struct *work)
 	/* header read in */
 	if ((!m->req) && (m->rc.offset == m->rc.capacity)) {
 		p9_debug(P9_DEBUG_TRANS, "got new header\n");
-
-		err = p9_parse_header(&m->rc, NULL, NULL, NULL, 0);
+		/* Header size */
+		m->rc.size = 7;
+		m->rc.capacity = m->client->msize;
+		err = p9_parse_header(&m->rc, &m->rc.size, NULL, NULL, 0);
 		if (err) {
 			p9_debug(P9_DEBUG_ERROR,
 				 "error parsing header: %d\n", err);
 			goto error;
 		}
 
-		if (m->rc.size >= m->client->msize) {
-			p9_debug(P9_DEBUG_ERROR,
-				 "requested packet size too big: %d\n",
-				 m->rc.size);
-			err = -EIO;
-			goto error;
-		}
-
 		p9_debug(P9_DEBUG_TRANS,
 			 "mux %p pkt: size: %d bytes tag: %d\n",
 			 m, m->rc.size, m->rc.tag);
@@ -360,7 +354,7 @@ static void p9_read_work(struct work_struct *work)
 			goto error;
 		}
 		m->rc.sdata = (char *)m->req->rc + sizeof(struct p9_fcall);
-		memcpy(m->rc.sdata, m->tmp_buf, m->rc.capacity);
+		memcpy(m->rc.sdata, m->tmp_buf, 7);
 		m->rc.capacity = m->rc.size;
 	}
 
@@ -369,6 +363,7 @@ static void p9_read_work(struct work_struct *work)
 	 */
 	if ((m->req) && (m->rc.offset == m->rc.capacity)) {
 		p9_debug(P9_DEBUG_TRANS, "got new packet\n");
+		m->req->rc->size = m->rc.offset;
 		spin_lock(&m->client->lock);
 		if (m->req->status != REQ_STATUS_ERROR)
 			status = REQ_STATUS_RCVD;
diff --git a/net/9p/trans_rdma.c b/net/9p/trans_rdma.c
index 3d414acb7015..002badbcc9c0 100644
--- a/net/9p/trans_rdma.c
+++ b/net/9p/trans_rdma.c
@@ -319,7 +319,7 @@ recv_done(struct ib_cq *cq, struct ib_wc *wc)
 
 	if (wc->status != IB_WC_SUCCESS)
 		goto err_out;
-
+	c->rc->size = wc->byte_len;
 	err = p9_parse_header(c->rc, NULL, NULL, &tag, 1);
 	if (err)
 		goto err_out;
diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
index 05006cbb3361..6d515f7ebfaf 100644
--- a/net/9p/trans_virtio.c
+++ b/net/9p/trans_virtio.c
@@ -159,8 +159,10 @@ static void req_done(struct virtqueue *vq)
 		spin_unlock_irqrestore(&chan->lock, flags);
 		/* Wakeup if anyone waiting for VirtIO ring space. */
 		wake_up(chan->vc_wq);
-		if (len)
+		if (len) {
+			req->rc->size = len;
 			p9_client_cb(chan->client, req, REQ_STATUS_RCVD);
+		}
 	}
 }
 
@@ -446,7 +448,7 @@ p9_virtio_zc_request(struct p9_client *client, struct p9_req_t *req,
 		out += pack_sg_list_p(chan->sg, out, VIRTQUEUE_NUM,
 				      out_pages, out_nr_pages, offs, outlen);
 	}
-		
+
 	/*
 	 * Take care of in data
 	 * For example TREAD have 11.
-- 
2.11.0

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ