On Tue, Nov 17, 2015 at 08:39:30AM -0800, Rich Lane wrote: > > I don't think that adding a SIGINT handler is the right solution, though. The > guest app could be killed with another signal (SIGKILL).
Good point. > Worse, a malicious or > buggy guest could write to just that field. vhost should not crash no matter > what the guest writes into the virtqueues. Yeah, I agree with you: though we could fix this issue in the source side, we also should do some defend here. How about following patch then? Note that the vec_id overflow check should be done before referencing it, but not after. Hence I moved it ahead. --yliu --- diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c index 9322ce6..08f5942 100644 --- a/lib/librte_vhost/vhost_rxtx.c +++ b/lib/librte_vhost/vhost_rxtx.c @@ -132,6 +132,8 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id, /* Get descriptor from available ring */ desc = &vq->desc[head[packet_success]]; + if (desc->len == 0) + break; buff = pkts[packet_success]; @@ -153,6 +155,8 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id, /* Buffer address translation. */ buff_addr = gpa_to_vva(dev, desc->addr); } else { + if (desc->len < vq->vhost_hlen) + break; vb_offset += vq->vhost_hlen; hdr = 1; } @@ -446,6 +450,9 @@ update_secure_len(struct vhost_virtqueue *vq, uint32_t id, uint32_t vec_id = *vec_idx; do { + if (vec_id >= BUF_VECTOR_MAX) + break; + next_desc = 0; len += vq->desc[idx].len; vq->buf_vec[vec_id].buf_addr = vq->desc[idx].addr; @@ -519,6 +526,8 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t queue_id, goto merge_rx_exit; } else { update_secure_len(vq, res_cur_idx, &secure_len, &vec_idx); + if (secure_len == 0) + goto merge_rx_exit; res_cur_idx++; } } while (pkt_len > secure_len); @@ -631,6 +640,8 @@ rte_vhost_dequeue_burst(struct virtio_net *dev, uint16_t queue_id, uint8_t alloc_err = 0; desc = &vq->desc[head[entry_success]]; + if (desc->len == 0) + break; /* Discard first buffer as it is the virtio header */ if (desc->flags & VRING_DESC_F_NEXT) { @@ -638,6 +649,8 @@ rte_vhost_dequeue_burst(struct virtio_net *dev, uint16_t queue_id, vb_offset = 0; vb_avail = desc->len; } else { + if (desc->len < vq->vhost_hlen) + break; vb_offset = vq->vhost_hlen; vb_avail = desc->len - vb_offset; }