Re: [PATCH] hw/i386/amd_iommu: Don't leak memory in amdvi_update_iotlb()

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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...

Oh now I remembered:
https://lore.kernel.org/qemu-devel/CACGkMEtjmpX8G9HYZ0r3n5ErhAENKhQ81f4ocfCYrh=XoF=5hw@mail.gmail.com/

> ---
>   hw/i386/amd_iommu.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> 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);
>