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-sta...@nongnu.org
> Reported-by: Alexander Bulekov <alx...@bu.edu>
> Fixes: f6ad2e32f8 ("ahci: add ahci emulation")
> BugLink: https://bugs.launchpad.net/qemu/+bug/1884693
> Signed-off-by: Philippe Mathieu-Daudé <f4...@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;
> }
>
