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

2015-11-19 Thread Yuanhan Liu
On Wed, Nov 18, 2015 at 11:15:25AM +, Xie, Huawei wrote: > On 11/18/2015 4:47 PM, Yuanhan Liu wrote: > > On Wed, Nov 18, 2015 at 07:53:24AM +, Xie, Huawei wrote: > > ... > >>> do { > >>> + if (vec_id >= BUF_VECTOR_MAX) > >>> + break; > >>> + > >>>

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

2015-11-18 Thread Yuanhan Liu
On Wed, Nov 18, 2015 at 07:53:24AM +, Xie, Huawei wrote: ... > > do { > > + if (vec_id >= BUF_VECTOR_MAX) > > + break; > > + > > next_desc = 0; > > len += vq->desc[idx].len; > > vq->buf_vec[vec_id].buf_addr =

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

2015-11-18 Thread Xie, Huawei
On 11/18/2015 11:53 PM, Stephen Hemminger wrote: > On Wed, 18 Nov 2015 06:13:08 + > "Xie, Huawei" wrote: > >> On 11/18/2015 10:56 AM, Yuanhan Liu wrote: >>> 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,

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

2015-11-18 Thread Yuanhan Liu
On Tue, Nov 17, 2015 at 09:26:57PM -0800, Rich Lane wrote: > On Tue, Nov 17, 2015 at 6:56 PM, Yuanhan Liu > wrote: > > @@ -519,6 +526,8 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t > queue_id, > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? goto merge_rx_exit; > ? ? ? ? ? ?

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

2015-11-18 Thread Xie, Huawei
On 11/18/2015 4:47 PM, Yuanhan Liu wrote: > On Wed, Nov 18, 2015 at 07:53:24AM +, Xie, Huawei wrote: > ... >>> do { >>> + if (vec_id >= BUF_VECTOR_MAX) >>> + break; >>> + >>> next_desc = 0; >>> len += vq->desc[idx].len; >>>

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

2015-11-18 Thread Yuanhan Liu
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

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

2015-11-18 Thread Xie, Huawei
On 11/18/2015 2:25 PM, Yuanhan Liu wrote: > On Wed, Nov 18, 2015 at 06:13:08AM +, Xie, Huawei wrote: >> On 11/18/2015 10:56 AM, Yuanhan Liu wrote: >>> 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.

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

2015-11-18 Thread Xie, Huawei
On 11/18/2015 10:56 AM, Yuanhan Liu wrote: > 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

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

2015-11-18 Thread Stephen Hemminger
On Wed, 18 Nov 2015 06:13:08 + "Xie, Huawei" wrote: > On 11/18/2015 10:56 AM, Yuanhan Liu wrote: > > 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

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

2015-11-18 Thread Xie, Huawei
On 11/18/2015 10:56 AM, Yuanhan Liu wrote: > 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

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

2015-11-18 Thread Wang, Zhihong
> -Original Message- > From: Yuanhan Liu [mailto:yuanhan.liu at linux.intel.com] > Sent: Wednesday, November 18, 2015 10:57 AM > To: Rich Lane > Cc: dev at dpdk.org; Xie, Huawei ; Wang, Zhihong > ; Richardson, Bruce > Subject: Re: [PATCH] vhost: avoid buffer overflow in

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

2015-11-17 Thread Rich Lane
On Tue, Nov 17, 2015 at 6:56 PM, Yuanhan Liu wrote: > @@ -519,6 +526,8 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t > queue_id, > goto merge_rx_exit; > } else { >

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

2015-11-17 Thread Yuanhan Liu
On Thu, Nov 12, 2015 at 01:46:03PM -0800, Rich Lane wrote: > You can reproduce this with l2fwd and the vhost PMD. > > You'll need this patch on top of the vhost PMD patches: > --- a/lib/librte_vhost/virtio-net.c > +++ b/lib/librte_vhost/virtio-net.c > @@ -471,7 +471,7 @@ reset_owner(struct

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

2015-11-17 Thread Rich Lane
On Tue, Nov 17, 2015 at 5:23 AM, Yuanhan Liu wrote: > On Thu, Nov 12, 2015 at 01:46:03PM -0800, Rich Lane wrote: > > You can reproduce this with l2fwd and the vhost PMD. > > > > You'll need this patch on top of the vhost PMD patches: > > --- a/lib/librte_vhost/virtio-net.c > > +++

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

2015-11-12 Thread Yuanhan Liu
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

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

2015-11-12 Thread Rich Lane
You can reproduce this with l2fwd and the vhost PMD. You'll need this patch on top of the vhost PMD patches: --- a/lib/librte_vhost/virtio-net.c +++ b/lib/librte_vhost/virtio-net.c @@ -471,7 +471,7 @@ reset_owner(struct vhost_device_ctx ctx) return -1; if (dev->flags &

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

2015-11-12 Thread Rich Lane
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