| Message ID | 1417067965-9159-1-git-send-email-jasowang@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, 11/27 13:59, Jason Wang wrote: > virtio_net_handle_ctrl() and other functions that process control vq > request call iov_discard_front() which will shorten the iov. This will > lead unmapping in virtqueue_push() leaks mapping. > > Fixes this by keeping the original iov untouched and using a temp variable > in those functions. I'm a little confused. Commit message mentioned "other functions" but in this patch only virtio_net_handle_ctrl is changed. So is this complete? Fam > > Cc: Wen Congyang <wency@cn.fujitsu.com> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > Cc: qemu-stable@nongnu.org > Signed-off-by: Jason Wang <jasowang@redhat.com> > --- > hw/net/virtio-net.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index 9b88775..fdb4edd 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -798,7 +798,7 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) > virtio_net_ctrl_ack status = VIRTIO_NET_ERR; > VirtQueueElement elem; > size_t s; > - struct iovec *iov; > + struct iovec *iov, *iov2; > unsigned int iov_cnt; > > while (virtqueue_pop(vq, &elem)) { > @@ -808,8 +808,12 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) > exit(1); > } > > - iov = elem.out_sg; > iov_cnt = elem.out_num; > + s = sizeof(struct iovec) * elem.out_num; > + iov = g_malloc(s); > + memcpy(iov, elem.out_sg, s); > + iov2 = iov; > + > s = iov_to_buf(iov, iov_cnt, 0, &ctrl, sizeof(ctrl)); > iov_discard_front(&iov, &iov_cnt, sizeof(ctrl)); > if (s != sizeof(ctrl)) { > @@ -833,6 +837,7 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) > > virtqueue_push(vq, &elem, sizeof(status)); > virtio_notify(vdev, vq); > + g_free(iov2); > } > } > > -- > 1.9.1 > >
On Thu, Nov 27, 2014 at 3:45 PM, Fam Zheng <famz@redhat.com> wrote: > On Thu, 11/27 13:59, Jason Wang wrote: >> virtio_net_handle_ctrl() and other functions that process control vq >> request call iov_discard_front() which will shorten the iov. This >> will >> lead unmapping in virtqueue_push() leaks mapping. >> >> Fixes this by keeping the original iov untouched and using a temp >> variable >> in those functions. > > I'm a little confused. Commit message mentioned "other functions" but > in this > patch only virtio_net_handle_ctrl is changed. So is this complete? > > Fam It's ok since all other functions were called by vitio_net_handle_ctrl() and it pass temp variable to those specific helpers to handle ctrl vq commands. > > >> >> Cc: Wen Congyang <wency@cn.fujitsu.com> >> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> >> Cc: qemu-stable@nongnu.org >> Signed-off-by: Jason Wang <jasowang@redhat.com> >> --- >> hw/net/virtio-net.c | 9 +++++++-- >> 1 file changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c >> index 9b88775..fdb4edd 100644 >> --- a/hw/net/virtio-net.c >> +++ b/hw/net/virtio-net.c >> @@ -798,7 +798,7 @@ static void virtio_net_handle_ctrl(VirtIODevice >> *vdev, VirtQueue *vq) >> virtio_net_ctrl_ack status = VIRTIO_NET_ERR; >> VirtQueueElement elem; >> size_t s; >> - struct iovec *iov; >> + struct iovec *iov, *iov2; >> unsigned int iov_cnt; >> >> while (virtqueue_pop(vq, &elem)) { >> @@ -808,8 +808,12 @@ static void >> virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) >> exit(1); >> } >> >> - iov = elem.out_sg; >> iov_cnt = elem.out_num; >> + s = sizeof(struct iovec) * elem.out_num; >> + iov = g_malloc(s); >> + memcpy(iov, elem.out_sg, s); >> + iov2 = iov; >> + >> s = iov_to_buf(iov, iov_cnt, 0, &ctrl, sizeof(ctrl)); >> iov_discard_front(&iov, &iov_cnt, sizeof(ctrl)); >> if (s != sizeof(ctrl)) { >> @@ -833,6 +837,7 @@ static void virtio_net_handle_ctrl(VirtIODevice >> *vdev, VirtQueue *vq) >> >> virtqueue_push(vq, &elem, sizeof(status)); >> virtio_notify(vdev, vq); >> + g_free(iov2); >> } >> } >> >> -- >> 1.9.1 >> >> >
On Thu, 11/27 13:59, Jason Wang wrote: > virtio_net_handle_ctrl() and other functions that process control vq > request call iov_discard_front() which will shorten the iov. This will > lead unmapping in virtqueue_push() leaks mapping. > > Fixes this by keeping the original iov untouched and using a temp variable > in those functions. > > Cc: Wen Congyang <wency@cn.fujitsu.com> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > Cc: qemu-stable@nongnu.org > Signed-off-by: Jason Wang <jasowang@redhat.com> > --- > hw/net/virtio-net.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index 9b88775..fdb4edd 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -798,7 +798,7 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) > virtio_net_ctrl_ack status = VIRTIO_NET_ERR; > VirtQueueElement elem; > size_t s; > - struct iovec *iov; > + struct iovec *iov, *iov2; > unsigned int iov_cnt; > > while (virtqueue_pop(vq, &elem)) { > @@ -808,8 +808,12 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) > exit(1); > } > > - iov = elem.out_sg; > iov_cnt = elem.out_num; > + s = sizeof(struct iovec) * elem.out_num; > + iov = g_malloc(s); > + memcpy(iov, elem.out_sg, s); This could be iov = g_memdup(elem.out_sg, sizeof(struct iovect) * elem.out_num); Fam > + iov2 = iov; > + > s = iov_to_buf(iov, iov_cnt, 0, &ctrl, sizeof(ctrl)); > iov_discard_front(&iov, &iov_cnt, sizeof(ctrl)); > if (s != sizeof(ctrl)) { > @@ -833,6 +837,7 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) > > virtqueue_push(vq, &elem, sizeof(status)); > virtio_notify(vdev, vq); > + g_free(iov2); > } > } > > -- > 1.9.1 > >
On 11/27/2014 05:08 PM, Fam Zheng wrote: > On Thu, 11/27 13:59, Jason Wang wrote: >> > virtio_net_handle_ctrl() and other functions that process control vq >> > request call iov_discard_front() which will shorten the iov. This will >> > lead unmapping in virtqueue_push() leaks mapping. >> > >> > Fixes this by keeping the original iov untouched and using a temp variable >> > in those functions. >> > >> > Cc: Wen Congyang <wency@cn.fujitsu.com> >> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> >> > Cc: qemu-stable@nongnu.org >> > Signed-off-by: Jason Wang <jasowang@redhat.com> >> > --- >> > hw/net/virtio-net.c | 9 +++++++-- >> > 1 file changed, 7 insertions(+), 2 deletions(-) >> > >> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c >> > index 9b88775..fdb4edd 100644 >> > --- a/hw/net/virtio-net.c >> > +++ b/hw/net/virtio-net.c >> > @@ -798,7 +798,7 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) >> > virtio_net_ctrl_ack status = VIRTIO_NET_ERR; >> > VirtQueueElement elem; >> > size_t s; >> > - struct iovec *iov; >> > + struct iovec *iov, *iov2; >> > unsigned int iov_cnt; >> > >> > while (virtqueue_pop(vq, &elem)) { >> > @@ -808,8 +808,12 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) >> > exit(1); >> > } >> > >> > - iov = elem.out_sg; >> > iov_cnt = elem.out_num; >> > + s = sizeof(struct iovec) * elem.out_num; >> > + iov = g_malloc(s); >> > + memcpy(iov, elem.out_sg, s); > This could be > > iov = g_memdup(elem.out_sg, sizeof(struct iovect) * elem.out_num); > > Fam > Right, will post V2. Thanks
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 9b88775..fdb4edd 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -798,7 +798,7 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) virtio_net_ctrl_ack status = VIRTIO_NET_ERR; VirtQueueElement elem; size_t s; - struct iovec *iov; + struct iovec *iov, *iov2; unsigned int iov_cnt; while (virtqueue_pop(vq, &elem)) { @@ -808,8 +808,12 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) exit(1); } - iov = elem.out_sg; iov_cnt = elem.out_num; + s = sizeof(struct iovec) * elem.out_num; + iov = g_malloc(s); + memcpy(iov, elem.out_sg, s); + iov2 = iov; + s = iov_to_buf(iov, iov_cnt, 0, &ctrl, sizeof(ctrl)); iov_discard_front(&iov, &iov_cnt, sizeof(ctrl)); if (s != sizeof(ctrl)) { @@ -833,6 +837,7 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) virtqueue_push(vq, &elem, sizeof(status)); virtio_notify(vdev, vq); + g_free(iov2); } }
virtio_net_handle_ctrl() and other functions that process control vq request call iov_discard_front() which will shorten the iov. This will lead unmapping in virtqueue_push() leaks mapping. Fixes this by keeping the original iov untouched and using a temp variable in those functions. Cc: Wen Congyang <wency@cn.fujitsu.com> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Cc: qemu-stable@nongnu.org Signed-off-by: Jason Wang <jasowang@redhat.com> --- hw/net/virtio-net.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)