16/01/25 14:32, Peter Maydell пишет:
On Thu, 16 Jan 2025 at 11:17, Artem Nasonov <anaso...@astralinux.ru> wrote:
This assert was found during fuzzing and can be triggered with some qtest
commands.
So instead of assert failure I suggest to handle this error and abort the
command.
This patch is required at least to improve fuzzing process and do not spam with
this assert.
RFC.
Found by Linux Verification Center (linuxtesting.org) with libFuzzer.
Fixes: ed78352a59 ("ide: Fix incorrect handling of some PRDTs in ide_dma_cb()")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2777
Signed-off-by: Artem Nasonov <anaso...@astralinux.ru>
---
hw/ide/core.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index f9baba59e9..baca7121ec 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -931,7 +931,10 @@ static void ide_dma_cb(void *opaque, int ret)
s->io_buffer_size = n * 512;
prep_size = s->bus->dma->ops->prepare_buf(s->bus->dma, s->io_buffer_size);
/* prepare_buf() must succeed and respect the limit */
- assert(prep_size >= 0 && prep_size <= n * 512);
+ if (prep_size < 0 || prep_size > n * 512) {
+ ide_dma_error(s);
+ return;
+ }
Now the comment and the code disagree (the comment
says that the callback must never do the thing that we
now have code to handle).
What's the actual situation when the prepare_buf callback hits
this assertion? Is the problem in this code, or is it in the
callback implementation? Which IDEDMAOps is involved?
thanks
-- PMM
Steps to reproduse are described in related issue:
https://gitlab.com/qemu-project/qemu/-/issues/2777 In this case,
function ahci_dma_prepare_buf() from hw/ide/ahci.c stands for
s->bus->dma->ops->prepare_buf. It is called and returns -1. This is
because of call to ahci_populate_sglist() function, which returns -1 due
to check if (!prdtl), where prdtl is one of the fields of AHCIDevice cmd
header. So we have a situation: prepare_buf() must succeed, but returns
-1 for some reason and application fails (which is harmful for fuzzing
too). We may solve it in two ways: patch callback or patch caller. I
don't see any possible way to handle error of populating sglist inside
ahci_dma_prepare_buf() function: it fails to prepare buf, but has to
return something. Since then, we should catch this error and interrupt
operation or maybe other action. Thanks, Artem
