Re: [PATCH 1/2] softmmu/physmem: move last_ram_page() call under qemu_mutex_lock_ramlist()

[Peter Xu <peterx@redhat.com>]

On Fri, Mar 25, 2022 at 06:40:12PM +0300, Andrey Ryabinin wrote:
> The 'ram_list.blocks' modifications protected by 'ram_list.mutex'.
> last_ram_page() uses state of 'ram_list.blocks' to identify ram's size.
> ram_block_add() calls last_ram_page() before the mutex lock
> making the following race possible:
> 
>      CPU#0                                       CPU#1
>                                       ram_block_add()
>                                          old_ram_size = last_ram_page()
>                                          qemu_mutex_lock_ramlist()
>                                          ...
>                                          dirty_memory_extend(old_ram_size, new_ram_size);
> ram_block_add()
>    old_ram_size = last_ram_page()
> 
> 					      //insert block to ram_list
> 					      QLIST_INSERT_*_RCU()
> 					      qemu_mutex_unlock_ramlist()
>    qemu_mutex_lock_ramlist()
>    ....
>    dirty_memory_extend(old_ram_size, new_ram_size);
> 
> Such race may result in leaking some dirty memory bitmaps.
> 
> Because of stale 'old_ram_size' value, the dirty_memory_extend() on CPU#0
> will allocate and reinitialize some of the already allocated on CPU#1
> dirty memory bitmap blocks.
> 
> Fix this by moving last_ram_page() call under the qemu_mutex_lock_ramlist()
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Andrey Ryabinin <arbn@yandex-team.com>

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu