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

[Marc-André Lureau]

Hi

On Fri, Jun 23, 2017 at 6:28 AM Jia-Shiun Li <address@hidden> wrote:

> In commit 9e0bc24f dev->log_size was reset to zero too early before
> syncing vhost log. It causes syncing to be skipped.
>
>
ooch, I guess I didn't realize it was also accessing dev->log_size when
taking dev->log in local variable.

I wonder why the code is written this way, it looks like the function may
be reentered. For consistency, and perhaps for the reentering case, I would
use a local log_size variable too.

Btw, how did you find this regression?


> Move it to clear dev->log* after use.
>
> Signed-off-by: Jia-Shiun Li <address@hidden>
>
---
>  hw/virtio/vhost.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 6eddb09..c9ddf11 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -375,8 +375,6 @@ static void vhost_log_put(struct vhost_dev *dev, bool
> sync)
>      if (!log) {
>          return;
>      }
> -    dev->log = NULL;
> -    dev->log_size = 0;
>
>      --log->refcnt;
>      if (log->refcnt == 0) {
> @@ -396,6 +394,8 @@ static void vhost_log_put(struct vhost_dev *dev, bool
> sync)
>
>          g_free(log);
>      }
> +    dev->log = NULL;
> +    dev->log_size = 0;
>  }
>
>  static bool vhost_dev_log_is_shared(struct vhost_dev *dev)
> --
> 2.7.4
>
>
> --
Marc-André Lureau