Hi Jason,
On 9/14/23 05:46, Jason Wang wrote:
> On Wed, Sep 13, 2023 at 3:47 PM Eric Auger <eric.au...@redhat.com> wrote:
>> In vhost_commit(), it may happen that dev->mem_sections and
>> dev->tmp_sections are equal, in which case, unconditionally
>> freeing old_sections at the end of the function will also free
>> dev->mem_sections used on subsequent call leading to a segmentation
>> fault.
>>
>> Check this situation before deallocating memory.
>>
>> Signed-off-by: Eric Auger <eric.au...@redhat.com>
>> Fixes: c44317efecb2 ("vhost: Build temporary section list and deref
>> after commit")
>> CC: QEMU Stable <qemu-sta...@nongnu.org>
>>
>> ---
>>
>> This SIGSEV condition can be reproduced with
>> https://lore.kernel.org/all/20230904080451.424731-1-eric.au...@redhat.com/#r
>> This is most probably happening in a situation where the memory API is
>> used in a wrong manner but well.
> Any chance to move this to the memory API or we may end up with things
> like this in another listener?
I am not very familiar with the vhost code but aren't those tmp_sections
and mem_sections really specific to the vhost device? I am not sure we
can easily generalize.
Thanks
Eric
>
> Thanks
>
>> ---
>> hw/virtio/vhost.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>> index e2f6ffb446..c02c599ef0 100644
>> --- a/hw/virtio/vhost.c
>> +++ b/hw/virtio/vhost.c
>> @@ -545,6 +545,11 @@ static void vhost_commit(MemoryListener *listener)
>> dev->mem_sections = dev->tmp_sections;
>> dev->n_mem_sections = dev->n_tmp_sections;
>>
>> + if (old_sections == dev->mem_sections) {
>> + assert(n_old_sections == dev->n_mem_sections);
>> + return;
>> + }
>> +
>> if (dev->n_mem_sections != n_old_sections) {
>> changed = true;
>> } else {
>> --
>> 2.41.0
>>
