Commit 024e0e90 authored by Dariusz Stojaczyk's avatar Dariusz Stojaczyk Committed by Jim Harris
Browse files

vhost: check against virtio descriptor table overflow 



Also squashed function has_next_desc
into get_next_desc to simplify the
code.

We can't just mask indexes with
(desc_table_size - 1), since in
indirect descriptors case
desc_table_size might not be a
power of 2.

Change-Id: I8053b0e37c553548d76c7a9cfe6b4dbc11c28cfc
Signed-off-by: default avatarDariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/373744


Tested-by: default avatarSPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Reviewed-by: default avatarDaniel Verkamp <daniel.verkamp@intel.com>
parent 00e59e3b

lib/vhost/vhost.c

+30 −13
Original line number Diff line number Diff line
@@ -114,11 +114,19 @@ spdk_vhost_vq_avail_ring_get(struct rte_vhost_vring *vq, uint16_t *reqs, uint16_ 
	return count;

}



struct vring_desc *

spdk_vhost_vq_get_desc(struct rte_vhost_vring *vq, uint16_t req_idx)

int

spdk_vhost_vq_get_desc(struct rte_vhost_vring *vq, uint16_t req_idx, struct vring_desc **desc,

		       struct vring_desc **desc_table, uint32_t *desc_table_size)

{

	assert(req_idx < vq->size);

	return &vq->desc[req_idx];

	if (spdk_unlikely(req_idx >= vq->size)) {

		return -1;

	}



	*desc = &vq->desc[req_idx];

	*desc_table = vq->desc;

	*desc_table_size = vq->size;



	return 0;

}



/*
@@ -156,17 +164,26 @@ spdk_vhost_vq_used_ring_enqueue(struct spdk_vhost_dev *vdev, struct rte_vhost_vr 
	}

}



bool

spdk_vhost_vring_desc_has_next(struct vring_desc *cur_desc)

int

spdk_vhost_vring_desc_get_next(struct vring_desc **desc,

			       struct vring_desc *desc_table, uint32_t desc_table_size)

{

	return !!(cur_desc->flags & VRING_DESC_F_NEXT);

	struct vring_desc *old_desc = *desc;

	uint16_t next_idx;



	if ((old_desc->flags & VRING_DESC_F_NEXT) == 0) {

		*desc = NULL;

		return 0;

	}



struct vring_desc *

spdk_vhost_vring_desc_get_next(struct vring_desc *vq_desc, struct vring_desc *cur_desc)

{

	assert(spdk_vhost_vring_desc_has_next(cur_desc));

	return &vq_desc[cur_desc->next];

	next_idx = old_desc->next;

	if (spdk_unlikely(next_idx >= desc_table_size)) {

		*desc = NULL;

		return -1;

	}



	*desc = &desc_table[next_idx];

	return 0;

}



bool

lib/vhost/vhost_blk.c

+15 −5
Original line number Diff line number Diff line
@@ -125,9 +125,16 @@ static int 
blk_iovs_setup(struct spdk_vhost_dev *vdev, struct rte_vhost_vring *vq, uint16_t req_idx,

	       struct iovec *iovs, uint16_t *iovs_cnt, uint32_t *length)

{

	struct vring_desc *desc = spdk_vhost_vq_get_desc(vq, req_idx);

	struct vring_desc *desc, *desc_table;

	uint16_t out_cnt = 0, cnt = 0;

	uint32_t len = 0;

	uint32_t desc_table_size, len = 0;

	int rc;



	rc = spdk_vhost_vq_get_desc(vq, req_idx, &desc, &desc_table, &desc_table_size);

	if (rc != 0) {

		SPDK_ERRLOG("%s: Invalid descriptor at index %"PRIu16".\n", vdev->name, req_idx);

		return -1;

	}



	while (1) {

		/*
@@ -150,9 +157,12 @@ blk_iovs_setup(struct spdk_vhost_dev *vdev, struct rte_vhost_vring *vq, uint16_t 


		out_cnt += spdk_vhost_vring_desc_is_wr(desc);



		if (spdk_vhost_vring_desc_has_next(desc)) {

			desc = spdk_vhost_vring_desc_get_next(vq->desc, desc);

		} else {

		rc = spdk_vhost_vring_desc_get_next(&desc, desc_table, desc_table_size);

		if (rc != 0) {

			SPDK_ERRLOG("%s: Descriptor chain at index %"PRIu16" terminated unexpectedly.\n",

				    vdev->name, req_idx);

			return -1;

		} else if (desc == NULL) {

			break;

		}

	}

lib/vhost/vhost_internal.h

+32 −5
Original line number Diff line number Diff line
@@ -128,13 +128,40 @@ uint16_t spdk_vhost_vq_avail_ring_get(struct rte_vhost_vring *vq, uint16_t *reqs 
				      uint16_t reqs_len);

bool spdk_vhost_vq_should_notify(struct spdk_vhost_dev *vdev, struct rte_vhost_vring *vq);



struct vring_desc *spdk_vhost_vq_get_desc(struct rte_vhost_vring *vq, uint16_t req_idx);



/**

 * Get a virtio descriptor at given index in given virtqueue.

 * The descriptor will provide access to the entire descriptor

 * chain. The subsequent descriptors are accesible via

 * \c spdk_vhost_vring_desc_get_next.

 * \param vq virtqueue

 * \param req_idx descriptor index

 * \param desc pointer to be set to the descriptor

 * \param desc_table descriptor table to be used with

 * \c spdk_vhost_vring_desc_get_next. This might be either

 * default virtqueue descriptor table or per-chain indirect

 * table.

 * \param desc_table_size size of the *desc_table*

 * \return 0 on success, -1 if given index is invalid.

 * If -1 is returned, the params won't be changed.

 */

int spdk_vhost_vq_get_desc(struct rte_vhost_vring *vq, uint16_t req_idx, struct vring_desc **desc,

			   struct vring_desc **desc_table, uint32_t *desc_table_size);

void spdk_vhost_vq_used_ring_enqueue(struct spdk_vhost_dev *vdev, struct rte_vhost_vring *vq,

				     uint16_t id, uint32_t len);

bool spdk_vhost_vring_desc_has_next(struct vring_desc *cur_desc);

struct vring_desc *spdk_vhost_vring_desc_get_next(struct vring_desc *vq_desc,

		struct vring_desc *cur_desc);



/**

 * Get subsequent descriptor from given table.

 * \param desc current descriptor, will be set to the

 * next descriptor (NULL in case this is the last

 * descriptor in the chain or the next desc is invalid)

 * \param desc_table descriptor table

 * \param desc_table_size size of the *desc_table*

 * \return 0 on success, -1 if given index is invalid

 * The *desc* param will be set regardless of the

 * return value.

 */

int spdk_vhost_vring_desc_get_next(struct vring_desc **desc,

				   struct vring_desc *desc_table, uint32_t desc_table_size);

bool spdk_vhost_vring_desc_is_wr(struct vring_desc *cur_desc);



int spdk_vhost_vring_desc_to_iov(struct spdk_vhost_dev *vdev, struct iovec *iov,

lib/vhost/vhost_scsi.c

+85 −49
Original line number Diff line number Diff line
@@ -174,10 +174,11 @@ eventq_enqueue(struct spdk_vhost_scsi_dev *svdev, unsigned scsi_dev_num, uint32_ 
	       uint32_t reason)

{

	struct rte_vhost_vring *vq;

	struct vring_desc *desc;

	struct vring_desc *desc, *desc_table;

	struct virtio_scsi_event *desc_ev;

	uint32_t req_size;

	uint32_t desc_table_size, req_size = 0;

	uint16_t req;

	int rc;



	assert(scsi_dev_num < SPDK_VHOST_SCSI_CTRLR_MAX_DEVS);
@@ -189,13 +190,21 @@ eventq_enqueue(struct spdk_vhost_scsi_dev *svdev, unsigned scsi_dev_num, uint32_ 
		return;

	}



	desc =  spdk_vhost_vq_get_desc(vq, req);

	desc_ev = spdk_vhost_gpa_to_vva(&svdev->vdev, desc->addr);

	rc = spdk_vhost_vq_get_desc(vq, req, &desc, &desc_table, &desc_table_size);

	if (rc != 0 || desc->len < sizeof(*desc_ev)) {

		SPDK_ERRLOG("Controller %s: Invalid eventq descriptor at index %"PRIu16".\n",

			    svdev->vdev.name, req);

		goto out;

	}



	desc_ev = spdk_vhost_gpa_to_vva(&svdev->vdev, desc->addr);

	if (desc->len < sizeof(*desc_ev) || desc_ev == NULL) {

		SPDK_ERRLOG("Controller %s: Invalid eventq descriptor.\n", svdev->vdev.name);

		SPDK_ERRLOG("Controller %s: Invalid eventq descriptor at index %"PRIu16".\n",

			    svdev->vdev.name, req);

		req_size = 0;

	} else {

		goto out;

	}



	desc_ev->event = event;

	desc_ev->lun[0] = 1;

	desc_ev->lun[1] = scsi_dev_num;
@@ -210,8 +219,8 @@ eventq_enqueue(struct spdk_vhost_scsi_dev *svdev, unsigned scsi_dev_num, uint32_ 
	memset(&desc_ev->lun[4], 0, 4);

	desc_ev->reason = reason;

	req_size = sizeof(*desc_ev);

	}



out:

	spdk_vhost_vq_used_ring_enqueue(&svdev->vdev, vq, req, req_size);

}
@@ -305,30 +314,42 @@ spdk_vhost_scsi_task_init_target(struct spdk_vhost_scsi_task *task, const __u8 * 
static void

process_ctrl_request(struct spdk_vhost_scsi_task *task)

{

	struct vring_desc *desc;

	struct vring_desc *desc, *desc_table;

	struct virtio_scsi_ctrl_tmf_req *ctrl_req;

	struct virtio_scsi_ctrl_an_resp *an_resp;

	uint32_t desc_table_size;

	int rc;



	spdk_scsi_task_construct(&task->scsi, spdk_vhost_scsi_task_mgmt_cpl, spdk_vhost_scsi_task_free_cb,

				 NULL);

	desc = spdk_vhost_vq_get_desc(task->vq, task->req_idx);

	rc = spdk_vhost_vq_get_desc(task->vq, task->req_idx, &desc, &desc_table, &desc_table_size);

	if (spdk_unlikely(rc != 0)) {

		SPDK_ERRLOG("%s: Invalid controlq descriptor at index %d.\n",

			    task->svdev->vdev.name, task->req_idx);

		goto out;

	}



	ctrl_req = spdk_vhost_gpa_to_vva(&task->svdev->vdev, desc->addr);



	SPDK_DEBUGLOG(SPDK_TRACE_VHOST_SCSI_QUEUE,

		      "Processing controlq descriptor: desc %d/%p, desc_addr %p, len %d, flags %d, last_used_idx %d; kickfd %d; size %d\n",

		      task->req_idx, desc, (void *)desc->addr, desc->len, desc->flags, task->vq->last_used_idx,

		      task->vq->kickfd, task->vq->size);

	SPDK_TRACEDUMP(SPDK_TRACE_VHOST_SCSI_QUEUE, "Request desriptor", (uint8_t *)ctrl_req,

	SPDK_TRACEDUMP(SPDK_TRACE_VHOST_SCSI_QUEUE, "Request descriptor", (uint8_t *)ctrl_req,

		       desc->len);



	spdk_vhost_scsi_task_init_target(task, ctrl_req->lun);



	spdk_vhost_vring_desc_get_next(&desc, desc_table, desc_table_size);

	if (spdk_unlikely(desc == NULL)) {

		SPDK_ERRLOG("%s: No response descriptor for controlq request %d.\n",

			    task->svdev->vdev.name, task->req_idx);

		goto out;

	}



	/* Process the TMF request */

	switch (ctrl_req->type) {

	case VIRTIO_SCSI_T_TMF:

		/* Get the response buffer */

		assert(spdk_vhost_vring_desc_has_next(desc));

		desc = spdk_vhost_vring_desc_get_next(task->vq->desc, desc);

		task->tmf_resp = spdk_vhost_gpa_to_vva(&task->svdev->vdev, desc->addr);



		/* Check if we are processing a valid request */
@@ -353,7 +374,6 @@ process_ctrl_request(struct spdk_vhost_scsi_task *task) 
		break;

	case VIRTIO_SCSI_T_AN_QUERY:

	case VIRTIO_SCSI_T_AN_SUBSCRIBE: {

		desc = spdk_vhost_vring_desc_get_next(task->vq->desc, desc);

		an_resp = spdk_vhost_gpa_to_vva(&task->svdev->vdev, desc->addr);

		an_resp->response = VIRTIO_SCSI_S_ABORTED;

		break;
@@ -363,6 +383,7 @@ process_ctrl_request(struct spdk_vhost_scsi_task *task) 
		break;

	}



out:

	spdk_vhost_vq_used_ring_enqueue(&task->svdev->vdev, task->vq, task->req_idx, 0);

	spdk_vhost_scsi_task_put(task);

}
@@ -377,15 +398,16 @@ static int 
task_data_setup(struct spdk_vhost_scsi_task *task,

		struct virtio_scsi_cmd_req **req)

{

	struct rte_vhost_vring *vq = task->vq;

	struct spdk_vhost_dev *vdev = &task->svdev->vdev;

	struct vring_desc *desc =  spdk_vhost_vq_get_desc(task->vq, task->req_idx);

	struct vring_desc *desc, *desc_table;

	struct iovec *iovs = task->iovs;

	uint16_t iovcnt = 0, iovcnt_max = SPDK_VHOST_IOVS_MAX;

	uint32_t len = 0;

	uint32_t desc_table_len, len = 0;

	int rc;



	/* Sanity check. First descriptor must be readable and must have next one. */

	if (spdk_unlikely(spdk_vhost_vring_desc_is_wr(desc) || !spdk_vhost_vring_desc_has_next(desc))) {

	rc = spdk_vhost_vq_get_desc(task->vq, task->req_idx, &desc, &desc_table, &desc_table_len);

	/* First descriptor must be readable */

	if (rc != 0 || spdk_unlikely(spdk_vhost_vring_desc_is_wr(desc))) {

		SPDK_WARNLOG("Invalid first (request) descriptor.\n");

		task->resp = NULL;

		goto abort_task;
@@ -394,7 +416,14 @@ task_data_setup(struct spdk_vhost_scsi_task *task, 
	spdk_scsi_task_construct(&task->scsi, spdk_vhost_scsi_task_cpl, spdk_vhost_scsi_task_free_cb, NULL);

	*req = spdk_vhost_gpa_to_vva(vdev, desc->addr);



	desc = spdk_vhost_vring_desc_get_next(vq->desc, desc);

	/* Each request must have at least 2 descriptors (e.g. request and response) */

	spdk_vhost_vring_desc_get_next(&desc, desc_table, desc_table_len);

	if (desc == NULL) {

		SPDK_WARNLOG("%s: Descriptor chain at index %d contains neither payload nor response buffer.\n",

			     vdev->name, task->req_idx);

		task->resp = NULL;

		goto abort_task;

	}

	task->scsi.dxfer_dir = spdk_vhost_vring_desc_is_wr(desc) ? SPDK_SCSI_DIR_FROM_DEV :

			       SPDK_SCSI_DIR_TO_DEV;

	task->scsi.iovs = iovs;
@@ -404,7 +433,16 @@ task_data_setup(struct spdk_vhost_scsi_task *task, 
		 * FROM_DEV (READ): [RD_req][WR_resp][WR_buf0]...[WR_bufN]

		 */

		task->resp = spdk_vhost_gpa_to_vva(vdev, desc->addr);

		if (!spdk_vhost_vring_desc_has_next(desc)) {



		rc = spdk_vhost_vring_desc_get_next(&desc, desc_table, desc_table_len);

		if (spdk_unlikely(rc != 0)) {

			SPDK_WARNLOG("%s: invalid descriptor chain at request index %d (descriptor id overflow?).\n",

				     vdev->name, task->req_idx);

			task->resp = NULL;

			goto abort_task;

		}



		if (desc == NULL) {

			/*

			 * TEST UNIT READY command and some others might not contain any payload and this is not an error.

			 */
@@ -418,22 +456,24 @@ task_data_setup(struct spdk_vhost_scsi_task *task, 
			return 0;

		}



		desc = spdk_vhost_vring_desc_get_next(vq->desc, desc);



		/* All remaining descriptors are data. */

		while (iovcnt < iovcnt_max) {

		while (desc && iovcnt < iovcnt_max) {

			if (spdk_unlikely(!spdk_vhost_vring_desc_is_wr(desc))) {

				SPDK_WARNLOG("FROM DEV cmd: descriptor nr %" PRIu16" in payload chain is read only.\n", iovcnt);

				task->resp = NULL;

				goto abort_task;

			}



			if (spdk_unlikely(spdk_vhost_vring_desc_to_iov(vdev, iovs, &iovcnt, desc))) {

				task->resp = NULL;

				goto abort_task;

			}

			len += desc->len;



			if (!spdk_vhost_vring_desc_has_next(desc))

				break;



			desc = spdk_vhost_vring_desc_get_next(vq->desc, desc);

			if (spdk_unlikely(!spdk_vhost_vring_desc_is_wr(desc))) {

				SPDK_WARNLOG("FROM DEV cmd: descriptor nr %" PRIu16" in payload chain is read only.\n", iovcnt);

			rc = spdk_vhost_vring_desc_get_next(&desc, desc_table, desc_table_len);

			if (spdk_unlikely(rc != 0)) {

				SPDK_WARNLOG("%s: invalid payload in descriptor chain starting at index %d.\n",

					     vdev->name, task->req_idx);

				task->resp = NULL;

				goto abort_task;

			}
@@ -453,19 +493,15 @@ task_data_setup(struct spdk_vhost_scsi_task *task, 
			}

			len += desc->len;



			if (!spdk_vhost_vring_desc_has_next(desc)) {

			spdk_vhost_vring_desc_get_next(&desc, desc_table, desc_table_len);

			if (spdk_unlikely(desc == NULL)) {

				SPDK_WARNLOG("TO_DEV cmd: no response descriptor.\n");

				task->resp = NULL;

				goto abort_task;

			}



			desc = spdk_vhost_vring_desc_get_next(vq->desc, desc);

		}



		task->resp = spdk_vhost_gpa_to_vva(vdev, desc->addr);

		if (spdk_vhost_vring_desc_has_next(desc)) {

			SPDK_WARNLOG("TO_DEV cmd: ignoring unexpected descriptors after response descriptor.\n");

		}

	}



	if (iovcnt == iovcnt_max) {

test/unit/lib/vhost/vhost_blk.c/vhost_blk_ut.c

+4 −5
Original line number Diff line number Diff line
@@ -63,14 +63,13 @@ DEFINE_STUB(spdk_ring_enqueue, size_t, (struct spdk_ring *ring, void **objs, siz 
DEFINE_STUB(spdk_ring_dequeue, size_t, (struct spdk_ring *ring, void **objs, size_t count), 0);

DEFINE_STUB_V(spdk_vhost_vq_used_ring_enqueue, (struct spdk_vhost_dev *vdev,

		struct rte_vhost_vring *vq, uint16_t id, uint32_t len));

DEFINE_STUB_P(spdk_vhost_vq_get_desc, struct vring_desc, (struct rte_vhost_vring *vq,

		uint16_t req_idx), {0});

DEFINE_STUB(spdk_vhost_vq_get_desc, int, (struct rte_vhost_vring *vq, uint16_t req_idx,

		struct vring_desc **desc, struct vring_desc **desc_table, uint32_t *desc_table_size), 0);

DEFINE_STUB(spdk_vhost_vring_desc_is_wr, bool, (struct vring_desc *cur_desc), false);

DEFINE_STUB(spdk_vhost_vring_desc_to_iov, int, (struct spdk_vhost_dev *vdev, struct iovec *iov,

		uint16_t *iov_index, const struct vring_desc *desc), 0);

DEFINE_STUB(spdk_vhost_vring_desc_has_next, bool, (struct vring_desc *cur_desc), false);

DEFINE_STUB_P(spdk_vhost_vring_desc_get_next, struct vring_desc, (struct vring_desc *vq_desc,

		struct vring_desc *cur_desc), {0});

DEFINE_STUB(spdk_vhost_vring_desc_get_next, int, (struct vring_desc **desc,

		struct vring_desc *desc_table, uint32_t desc_table_size), 0);

DEFINE_STUB(spdk_bdev_free_io, int, (struct spdk_bdev_io *bdev_io), 0);

DEFINE_STUB(spdk_bdev_readv, int, (struct spdk_bdev_desc *desc, struct spdk_io_channel *ch,

				   struct iovec *iov, int iovcnt,