| Message ID | 20240731170019.3590563-1-peter.maydell@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/i386/amd_iommu: Don't leak memory in amdvi_update_iotlb() | expand |
On 31/7/24 19:00, Peter Maydell wrote: > In amdvi_update_iotlb() we will only put a new entry in the hash > table if to_cache.perm is not IOMMU_NONE. However we allocate the > memory for the new AMDVIIOTLBEntry and for the hash table key > regardless. This means that in the IOMMU_NONE case we will leak the > memory we alloacted. > > Move the allocations into the if() to the point where we know we're > going to add the item to the hash table. > > Cc: qemu-stable@nongnu.org > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2452 > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > Tested with 'make check' and 'make check-avocado' only, but the > bug and fix seem straightforward... > --- > hw/i386/amd_iommu.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c index 6d4fde72f9b..87643d28917 100644 --- a/hw/i386/amd_iommu.c +++ b/hw/i386/amd_iommu.c @@ -357,12 +357,12 @@ static void amdvi_update_iotlb(AMDVIState *s, uint16_t devid, uint64_t gpa, IOMMUTLBEntry to_cache, uint16_t domid) { - AMDVIIOTLBEntry *entry = g_new(AMDVIIOTLBEntry, 1); - uint64_t *key = g_new(uint64_t, 1); - uint64_t gfn = gpa >> AMDVI_PAGE_SHIFT_4K; - /* don't cache erroneous translations */ if (to_cache.perm != IOMMU_NONE) { + AMDVIIOTLBEntry *entry = g_new(AMDVIIOTLBEntry, 1); + uint64_t *key = g_new(uint64_t, 1); + uint64_t gfn = gpa >> AMDVI_PAGE_SHIFT_4K; + trace_amdvi_cache_update(domid, PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid), gpa, to_cache.translated_addr);
In amdvi_update_iotlb() we will only put a new entry in the hash table if to_cache.perm is not IOMMU_NONE. However we allocate the memory for the new AMDVIIOTLBEntry and for the hash table key regardless. This means that in the IOMMU_NONE case we will leak the memory we alloacted. Move the allocations into the if() to the point where we know we're going to add the item to the hash table. Cc: qemu-stable@nongnu.org Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2452 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- Tested with 'make check' and 'make check-avocado' only, but the bug and fix seem straightforward... --- hw/i386/amd_iommu.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)