Re: [PATCH v2] hw/ide: replace assert with proper error handling

Thomas Huth <[email protected]> Wed, 22 Jul 2026 12:58:06 +0200
Newsgroups org.nongnu.qemu-trivial,org.nongnu.qemu-devel
Message-ID <[email protected]>
On 22/07/2026 12.41, Fiona Ebner wrote:
> From: Artem Nasonov <[email protected]>
> 
> In ide_dma_cb(), the call to prepare_buf() might return a negative
> result and cause an assertion failure. This was found during fuzzing
> and can be triggered with some qtest commands. Replace the assert with
> proper error handling in case the result is negative, but keep the
> assert for failing to respect the limit upon success. If that happens,
> it is an implementation error.
> 
> Found by Linux Verification Center (linuxtesting.org) with libFuzzer.
> 
> Cc: [email protected]
> Fixes: ed78352a59 ("ide: Fix incorrect handling of some PRDTs in ide_dma_cb()")
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/2777
> Signed-off-by: Artem Nasonov <[email protected]>
> Link: https://lore.kernel.org/qemu-devel/[email protected]
> [FE: improve commit message
>       keep assert for failing to respect the limit]
> Signed-off-by: Fiona Ebner <[email protected]>
> ---
> 
> Changes in v2:
> * Update the code comment accordingly.
> * Keep the assert() when the limit is not respected upon success.
> 
>   hw/ide/core.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index f78b00220b..4c1ee19d8e 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -921,8 +921,12 @@ static void ide_dma_cb(void *opaque, int ret)
>       s->io_buffer_index = 0;
>       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) {
> +        ide_dma_error(s);
> +        return;
> +    }
> +    /* If prepare_buf() succeeds, it must respect the limit. */
> +    assert(prep_size <= n * 512);
>   
>       /*
>        * Now prep_size stores the number of bytes in the sglist, and

Thanks for the update!

Reviewed-by: Thomas Huth <[email protected]>