On 20/05/2026 10.27, Fiona Ebner wrote:
Am 19.05.26 um 3:52 PM schrieb Fiona Ebner:
Dear Maintainers,
Am 16.01.25 um 1:30 PM schrieb Артем Насонов:
16/01/25 14:32, Peter Maydell пишет:
On Thu, 16 Jan 2025 at 11:17, Artem Nasonov <[email protected]>
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 <[email protected]>
---
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
I'd like to ping this patch, since we got a report from a user hitting
this issue in production now [0].
I suppose we could keep the assert for the latter half, since not
respecting the limit would be an implementation error inside QEMU AFAIU.
I got a mail bounce for the original author's address, so please let me
know if I should send a v2 with the code comment updated and optionally
my suggestion with keeping the latter half of the assert folded in.
Hi Fiona,
seems like AI tools can find this problem, too:
https://gitlab.com/qemu-project/qemu/-/work_items/3901
so we should definitely fix this before we get swamped with more bug tickets
about this problem. Thus: Yes, please pick up the patch and send a v2 with
doing the ide_dma_error() for prep_size < 0 and doing the assert() for
prep_size > n * 512.
Thanks,
Thomas
