[Qemu-devel] [PATCH v2] vhost: Fix use-after-free in vhost_log_put()

Jia-Shiun Li posted 1 patch 6 years, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1498824184-1249-1-git-send-email-jsli@synology.com
Test FreeBSD passed
Test checkpatch passed
Test docker passed
Test s390x passed
hw/virtio/vhost.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
[Qemu-devel] [PATCH v2] vhost: Fix use-after-free in vhost_log_put()
Posted by Jia-Shiun Li 6 years, 9 months ago
In commit 9e0bc24f dev->log_size was reset to zero too early before
syncing vhost log. It causes syncing to be skipped.

Use local variable to keep its value before resetting.

Signed-off-by: Jia-Shiun Li <jsli@synology.com>
---

v1 -> v2:
 * Use local variable to keep value of dev->log_size.

---
 hw/virtio/vhost.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 6eddb09..c1c5714 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -371,6 +371,7 @@ static struct vhost_log *vhost_log_get(uint64_t size, bool share)
 static void vhost_log_put(struct vhost_dev *dev, bool sync)
 {
     struct vhost_log *log = dev->log;
+    uint64_t log_size = dev->log_size;
 
     if (!log) {
         return;
@@ -381,8 +382,8 @@ static void vhost_log_put(struct vhost_dev *dev, bool sync)
     --log->refcnt;
     if (log->refcnt == 0) {
         /* Sync only the range covered by the old log */
-        if (dev->log_size && sync) {
-            vhost_log_sync_range(dev, 0, dev->log_size * VHOST_LOG_CHUNK - 1);
+        if (log_size && sync) {
+            vhost_log_sync_range(dev, 0, log_size * VHOST_LOG_CHUNK - 1);
         }
 
         if (vhost_log == log) {
-- 
2.7.4


Re: [Qemu-devel] [PATCH v2] vhost: Fix use-after-free in vhost_log_put()
Posted by Marc-André Lureau 6 years, 9 months ago
On Fri, Jun 30, 2017 at 2:03 PM Jia-Shiun Li <jsli@synology.com> wrote:

> In commit 9e0bc24f dev->log_size was reset to zero too early before
> syncing vhost log. It causes syncing to be skipped.
>
> Use local variable to keep its value before resetting.
>
> Signed-off-by: Jia-Shiun Li <jsli@synology.com>
>

 Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

---
>
> v1 -> v2:
>  * Use local variable to keep value of dev->log_size.
>
> ---
>  hw/virtio/vhost.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 6eddb09..c1c5714 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -371,6 +371,7 @@ static struct vhost_log *vhost_log_get(uint64_t size,
> bool share)
>  static void vhost_log_put(struct vhost_dev *dev, bool sync)
>  {
>      struct vhost_log *log = dev->log;
> +    uint64_t log_size = dev->log_size;
>
>      if (!log) {
>          return;
> @@ -381,8 +382,8 @@ static void vhost_log_put(struct vhost_dev *dev, bool
> sync)
>      --log->refcnt;
>      if (log->refcnt == 0) {
>          /* Sync only the range covered by the old log */
> -        if (dev->log_size && sync) {
> -            vhost_log_sync_range(dev, 0, dev->log_size * VHOST_LOG_CHUNK
> - 1);
> +        if (log_size && sync) {
> +            vhost_log_sync_range(dev, 0, log_size * VHOST_LOG_CHUNK - 1);
>          }
>
>          if (vhost_log == log) {
> --
> 2.7.4
>
> --
Marc-André Lureau