[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] vhost: Add a defensive check in vhost_commit against wrong d
From: |
Jason Wang |
Subject: |
Re: [PATCH] vhost: Add a defensive check in vhost_commit against wrong deallocation |
Date: |
Thu, 14 Sep 2023 11:46:25 +0800 |
On Wed, Sep 13, 2023 at 3:47 PM Eric Auger <eric.auger@redhat.com> wrote:
>
> In vhost_commit(), it may happen that dev->mem_sections and
> dev->tmp_sections are equal, in which case, unconditionally
> freeing old_sections at the end of the function will also free
> dev->mem_sections used on subsequent call leading to a segmentation
> fault.
>
> Check this situation before deallocating memory.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Fixes: c44317efecb2 ("vhost: Build temporary section list and deref
> after commit")
> CC: QEMU Stable <qemu-stable@nongnu.org>
>
> ---
>
> This SIGSEV condition can be reproduced with
> 20230904080451.424731-1-eric.auger@redhat.com/#r">https://lore.kernel.org/all/20230904080451.424731-1-eric.auger@redhat.com/#r
> This is most probably happening in a situation where the memory API is
> used in a wrong manner but well.
Any chance to move this to the memory API or we may end up with things
like this in another listener?
Thanks
> ---
> hw/virtio/vhost.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index e2f6ffb446..c02c599ef0 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -545,6 +545,11 @@ static void vhost_commit(MemoryListener *listener)
> dev->mem_sections = dev->tmp_sections;
> dev->n_mem_sections = dev->n_tmp_sections;
>
> + if (old_sections == dev->mem_sections) {
> + assert(n_old_sections == dev->n_mem_sections);
> + return;
> + }
> +
> if (dev->n_mem_sections != n_old_sections) {
> changed = true;
> } else {
> --
> 2.41.0
>