virtio-net: fix unmap leak

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 <[email protected]>

Cc Stefano Stabellini <[email protected]>

Cc [email protected]

Signed-off-by Jason Wang <[email protected]>

---
 hw/net/virtio-net.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

hw/net/virtio-net.c +7 -2

index 9b88775..fdb4edd 100644
@@ -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);
     }
 }
 

Signature · 1 line

1.9.1

Quoted 5 lines · Jason Wang

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.

Quoted 3 lines

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

Quoted 53 lines

Cc Wen Congyang <[email protected]>

Cc Stefano Stabellini <[email protected]>

Cc [email protected]

Signed-off-by Jason Wang <[email protected]>

---

  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

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 <[email protected]>

Cc Stefano Stabellini <[email protected]>

Cc [email protected]

Signed-off-by Jason Wang <[email protected]>

---
 hw/net/virtio-net.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

hw/net/virtio-net.c +1 -1

index 9b88775..fdb4edd 100644
@@ -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)) {

hw/net/virtio-net.c +3 -0

+        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);
     }
 }
 

Signature · 1 line

1.9.1

Quoted 32 lines · Jason Wang

Quoted 32 lines

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 <[email protected]>

Cc Stefano Stabellini <[email protected]>

Cc [email protected]

Signed-off-by Jason Wang <[email protected]>

---
 hw/net/virtio-net.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

hw/net/virtio-net.c +1 -1

index 9b88775..fdb4edd 100644
@@ -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);
         }
 

Quoted 5 lines

hw/net/virtio-net.c static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) · line 814

Quoted 5 lines

hw/net/virtio-net.c +3 -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