Re: Fix a potential Use-after-free in virtio_iommu_handle_command() (v6.2.0).

From: Thomas Huth <thuth@redhat.com>
To: wliang@stu.xidian.edu.cn, Eric Auger <eric.auger@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>
Cc: "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>
Subject: Re: Fix a potential Use-after-free in virtio_iommu_handle_command() (v6.2.0).
Date: Wed, 23 Feb 2022 17:02:17 +0100	[thread overview]
Message-ID: <3ce08bdb-fecd-549a-5c09-0b5fa65de4ba@redhat.com> (raw)
In-Reply-To: <1b79118e.25c5.17f2702b9d5.Coremail.wliang@stu.xidian.edu.cn>

On 23/02/2022 15.36, wliang@stu.xidian.edu.cn wrote:
> Hi all,
> 
> I find a potential Use-after-free in QEMU 6.2.0, which is in 
> virtio_iommu_handle_command() (./hw/virtio/virtio-iommu.c).
> 
> Specifically, in the loop body, the variable 'buf' allocated at line 639 can 
> be freed by g_free() at line 659. However, if the execution path enters the 
> loop body again and the if branch takes true at line 616, the control will 
> directly jump to 'out' at line 651. At this time, 'buf' is a freed pointer, 
> which is not assigned with an allocated memory but used at line 653. As a 
> result, a UAF bug is triggered.
> 
> 
> 
> 599    for (;;) {
> ...
> 615        sz = iov_to_buf(iov, iov_cnt, 0, &head, sizeof(head));
> 616        if (unlikely(sz != sizeof(head))) {
> 617            tail.status = VIRTIO_IOMMU_S_DEVERR;
> 618            goto out;
> 619        }
> ...
> 639            buf = g_malloc0(output_size);
> ...
> 651out:
> 652        sz = iov_from_buf(elem->in_sg, elem->in_num, 0,
> 653                          buf ? buf : &tail, output_size);
> ...
> 659        g_free(buf);
> 660    }
> 
> 
> We can fix it by set ‘buf‘ to NULL after freeing it:
> 
> 
> 
> 651out:
> 652        sz = iov_from_buf(elem->in_sg, elem->in_num, 0,
> 653                          buf ? buf : &tail, output_size);
> ...
> 659        g_free(buf);
> +++buf = NULL;
> 660    }
> 
> 
> I'm looking forward to your confirmation.

  Hi,

thanks for your report and patch - but to make sure that the right people 
get attention, please use the scripts/get_maintainer.pl script to get a list 
of people who should be on CC:, or look into the MAINTAINERS file directly 
(for the next time - this time, I've CC:ed them now already).

  Thanks,
   Thomas