Hi, On 2/23/22 5:02 PM, Thomas Huth wrote: > On 23/02/2022 15.36, wli...@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. Thank you for the report. Yes setting buff to null after the g_free looks the right thing to do here. Please feel free to send the patch. > > 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 you Thomas for the cc ;-)
Eric > > Thanks, > Thomas >