Re: [PATCH v2] softmmu/physmem: fix memory leak in dirty_memory_extend()

[Peter Xu]

On Wed, Aug 28, 2024 at 11:07:43AM +0200, David Hildenbrand wrote:
> As reported by Peter, we might be leaking memory when removing the
> highest RAMBlock (in the weird ram_addr_t space), and adding a new one.
> 
> We will fail to realize that we already allocated bitmaps for more
> dirty memory blocks, and effectively discard the pointers to them.
> 
> Fix it by getting rid of last_ram_page() and by remembering the number
> of dirty memory blocks that have been allocated already.
> 
> While at it, let's use "unsigned int" for the number of blocks, which
> should be sufficient until we reach ~32 exabytes.
> 
> Looks like this leak was introduced as we switched from using a single
> bitmap_zero_extend() to allocating multiple bitmaps:
> bitmap_zero_extend() relies on g_renew() which should have taken care of
> this.
> 
> Resolves: 
> https://lkml.kernel.org/r/CAFEAcA-k7a+VObGAfCFNygQNfCKL=AfX6A4kScq=VSSK0peqPg@mail.gmail.com
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Fixes: 5b82b703b69a ("memory: RCU ram_list.dirty_memory[] for safe RAM 
> hotplug")
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Reviewed-by: Peter Xu <peterx@redhat.com>
> Tested-by: Peter Maydell <peter.maydell@linaro.org>
> Cc: qemu-stable@nongnu.org
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> 
> v1 -> v2:
> * Move the counter to RAMList
> * Use "unsigned int" instead of "ram_addr_t" as type for the number of
>   blocks

Thanks.  I'll pick this in the next pull if nobody disagrees (or beat me to
it).

-- 
Peter Xu