Re: [PATCH-for-5.1] hw/ide/ahci: Do not dma_memory_unmap(NULL)

[Philippe Mathieu-Daudé]

On 7/18/20 9:28 AM, Philippe Mathieu-Daudé wrote:
> libFuzzer triggered the following assertion:
> 
>   cat << EOF | qemu-system-i386 -M pc-q35-5.0 \
>     -nographic -monitor none -serial none -qtest stdio
>   outl 0xcf8 0x8000fa24
>   outl 0xcfc 0xe1068000
>   outl 0xcf8 0x8000fa04
>   outw 0xcfc 0x7
>   outl 0xcf8 0x8000fb20
>   write 0xe1068304 0x1 0x21
>   write 0xe1068318 0x1 0x21
>   write 0xe1068384 0x1 0x21
>   write 0xe1068398 0x2 0x21
>   EOF
>   qemu-system-i386: exec.c:3621: address_space_unmap: Assertion `mr != NULL' 
> failed.
>   Aborted (core dumped)
> 
> This is because we don't check the return value from dma_memory_map()
> which can return NULL, then we call dma_memory_unmap(NULL) which is
> illegal. Fix by only unmap if the value is not NULL (and the size is
> not the expected one).

Maybe worth mentioning it was hidden before but got revealed by commit
77f55eac6c ("exec: set map length to zero when returning NULL").

Cc'ing commit 77f55eac6c's reviewers.

> Cc: qemu-stable@nongnu.org
> Reported-by: Alexander Bulekov <alxndr@bu.edu>
> Fixes: f6ad2e32f8 ("ahci: add ahci emulation")
> BugLink: https://bugs.launchpad.net/qemu/+bug/1884693
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/ide/ahci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 009120f88b..4f596cb9ce 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -250,7 +250,7 @@ static void map_page(AddressSpace *as, uint8_t **ptr, 
> uint64_t addr,
>      }
>  
>      *ptr = dma_memory_map(as, addr, &len, DMA_DIRECTION_FROM_DEVICE);
> -    if (len < wanted) {
> +    if (len < wanted && *ptr) {
>          dma_memory_unmap(as, *ptr, len, DMA_DIRECTION_FROM_DEVICE, len);
>          *ptr = NULL;
>      }
>