Re: [PATCH] hw: ide: check the pointer before do dma memory unmap

[Philippe Mathieu-Daudé]

On 9/22/20 3:34 AM, Alexander Bulekov wrote:
> On 200815 0020, Li Qiang wrote:
>> In 'map_page' we need to check the return value of
>> 'dma_memory_map' to ensure the we actully maped something.
>> Otherwise, we will hit an assert in 'address_space_unmap'.
>> This is because we can't find the MR with the NULL buffer.
>> This is the LP#1884693:
>>
>> -->https://bugs.launchpad.net/qemu/+bug/1884693
>>
>> Reported-by: Alexander Bulekov <alxndr@bu.edu>
>> Signed-off-by: Li Qiang <liq3ea@163.com>
> 
> I'm not very familiar with the IDE code, but this seems like a simple
> null-ptr check, and Li has not received a response in over a month.

Yeah well it is not an easy bug... I spent few hours but at some
point it became too AHCI specific. I wanted to understand the bug
to answer the "Why do we get there?" "Can we get there with real
hardware?" questions, to be able to discern if this patch is OK,
or if it is hiding bugs and what we really use here is an assert().

> 
> Reviewed-by: Alexander Bulekov <alxndr@bu.edu>
> 
>> ---
>>  hw/ide/ahci.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
>> index 009120f88b..63e9fccdbe 100644
>> --- a/hw/ide/ahci.c
>> +++ b/hw/ide/ahci.c
>> @@ -250,6 +250,11 @@ static void map_page(AddressSpace *as, uint8_t **ptr, 
>> uint64_t addr,
>>      }
>>  
>>      *ptr = dma_memory_map(as, addr, &len, DMA_DIRECTION_FROM_DEVICE);
>> +
>> +    if (!*ptr) {
>> +        return;
>> +    }
>> +
>>      if (len < wanted) {
>>          dma_memory_unmap(as, *ptr, len, DMA_DIRECTION_FROM_DEVICE, len);
>>          *ptr = NULL;
>> -- 
>> 2.17.1
>>
>