diff mbox

[dpdk-dev] vhost: avoid buffer overflow in update_secure_len

Message ID 1447315353-42152-1-git-send-email-rlane@bigswitch.com (mailing list archive)
State Changes Requested, archived
Headers

Commit Message

Rich Lane Nov. 12, 2015, 8:02 a.m. UTC 
  The guest could trigger this buffer overflow by creating a cycle of descriptors
(which would also cause an infinite loop). The more common case is that
vq->avail->idx jumps out of the range [last_used_idx, last_used_idx+256). This
happens nearly every time when restarting a DPDK app inside a VM connected to a
vhost-user vswitch because the virtqueue memory allocated by the previous run
is zeroed.

Signed-off-by: Rich Lane <rlane@bigswitch.com>
---
 lib/librte_vhost/vhost_rxtx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Yuanhan Liu Nov. 12, 2015, 9:23 a.m. UTC | #1 
On Thu, Nov 12, 2015 at 12:02:33AM -0800, Rich Lane wrote:
> The guest could trigger this buffer overflow by creating a cycle of descriptors
> (which would also cause an infinite loop). The more common case is that
> vq->avail->idx jumps out of the range [last_used_idx, last_used_idx+256). This
> happens nearly every time when restarting a DPDK app inside a VM connected to a
> vhost-user vswitch because the virtqueue memory allocated by the previous run
> is zeroed.

Hi,

I somehow was aware of this issue before while reading the code.
Thinking that we never met that, I delayed the fix (it was still
in my TODO list).

Would you please tell me the steps (commands would be better) to
reproduce your issue? I'd like to know more about the isue: I'm
guessing maybe we need fix it with a bit more cares.

	--yliu
> 
> Signed-off-by: Rich Lane <rlane@bigswitch.com>
> ---
>  lib/librte_vhost/vhost_rxtx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c
> index 9322ce6..d95b478 100644
> --- a/lib/librte_vhost/vhost_rxtx.c
> +++ b/lib/librte_vhost/vhost_rxtx.c
> @@ -453,7 +453,7 @@ update_secure_len(struct vhost_virtqueue *vq, uint32_t id,
>  		vq->buf_vec[vec_id].desc_idx = idx;
>  		vec_id++;
>  
> -		if (vq->desc[idx].flags & VRING_DESC_F_NEXT) {
> +		if (vq->desc[idx].flags & VRING_DESC_F_NEXT && vec_id < BUF_VECTOR_MAX) {
>  			idx = vq->desc[idx].next;
>  			next_desc = 1;
>  		}
> -- 
> 1.9.1
diff mbox

Patch

diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c
index 9322ce6..d95b478 100644
--- a/lib/librte_vhost/vhost_rxtx.c
+++ b/lib/librte_vhost/vhost_rxtx.c
@@ -453,7 +453,7 @@  update_secure_len(struct vhost_virtqueue *vq, uint32_t id,
 		vq->buf_vec[vec_id].desc_idx = idx;
 		vec_id++;
 
-		if (vq->desc[idx].flags & VRING_DESC_F_NEXT) {
+		if (vq->desc[idx].flags & VRING_DESC_F_NEXT && vec_id < BUF_VECTOR_MAX) {
 			idx = vq->desc[idx].next;
 			next_desc = 1;
 		}