* [PATCH-for-5.1] hw/ide/ahci: Do not dma_memory_unmap(NULL)
@ 2020-07-18 7:28 Philippe Mathieu-Daudé
2020-07-21 17:34 ` Philippe Mathieu-Daudé
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-18 7:28 UTC (permalink / raw)
To: qemu-devel
Cc: Alexander Bulekov, John Snow, Philippe Mathieu-Daudé,
qemu-block, qemu-stable
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;
}
--
2.21.3
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH-for-5.1] hw/ide/ahci: Do not dma_memory_unmap(NULL)
2020-07-18 7:28 [PATCH-for-5.1] hw/ide/ahci: Do not dma_memory_unmap(NULL) Philippe Mathieu-Daudé
@ 2020-07-21 17:34 ` Philippe Mathieu-Daudé
2020-07-21 17:38 ` John Snow
2020-07-21 17:56 ` John Snow
2 siblings, 0 replies; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-21 17:34 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Prasad J Pandit, qemu-block, qemu-stable,
Alexander Bulekov, Paolo Bonzini, John Snow
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;
> }
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH-for-5.1] hw/ide/ahci: Do not dma_memory_unmap(NULL)
2020-07-18 7:28 [PATCH-for-5.1] hw/ide/ahci: Do not dma_memory_unmap(NULL) Philippe Mathieu-Daudé
2020-07-21 17:34 ` Philippe Mathieu-Daudé
@ 2020-07-21 17:38 ` John Snow
2020-07-21 17:56 ` John Snow
2 siblings, 0 replies; 4+ messages in thread
From: John Snow @ 2020-07-21 17:38 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Alexander Bulekov, qemu-stable, qemu-block
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.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH-for-5.1] hw/ide/ahci: Do not dma_memory_unmap(NULL)
2020-07-18 7:28 [PATCH-for-5.1] hw/ide/ahci: Do not dma_memory_unmap(NULL) Philippe Mathieu-Daudé
2020-07-21 17:34 ` Philippe Mathieu-Daudé
2020-07-21 17:38 ` John Snow
@ 2020-07-21 17:56 ` John Snow
2 siblings, 0 replies; 4+ messages in thread
From: John Snow @ 2020-07-21 17:56 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Alexander Bulekov, qemu-stable, qemu-block
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;
> }
>
Staged @ gitlab
https://gitlab.com/jsnow/qemu/-/commits/ide
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-07-21 17:57 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-07-18 7:28 [PATCH-for-5.1] hw/ide/ahci: Do not dma_memory_unmap(NULL) Philippe Mathieu-Daudé
2020-07-21 17:34 ` Philippe Mathieu-Daudé
2020-07-21 17:38 ` John Snow
2020-07-21 17:56 ` John Snow
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).