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

[John Snow]

On 7/18/20 3: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).
>
> 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;
>       }

Reviewed-by: John Snow <jsnow@redhat.com>

Thanks! this one wound up being easy.