Re: [Qemu-devel] [PATCH] [ide] Check that asynchronous (DMA) submission succeeds

[Anthony Liguori]

Ian Jackson wrote:
> Check that asynchronous (DMA) submission succeeds
>
> If it does not, abort the command immediately rather than dropping
> it on the floor.
>   

It's generally dangerous to call callbacks from the code that is issuing 
a bdrv_aio_ operation.  A malicious guest could potentially force the 
emulation into an infinite loop.

It would be better to use a bottom half to dispatch the callback.

Regards,

Anthony Liguori

> Signed-off-by: Ian Jackson <address@hidden>
> ---
>  hw/ide.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/hw/ide.c b/hw/ide.c
> index 1e60591..9b95c35 100644
> --- a/hw/ide.c
> +++ b/hw/ide.c
> @@ -737,6 +737,13 @@ static inline void ide_abort_command(IDEState *s)
>      s->status = READY_STAT | ERR_STAT;
>      s->error = ABRT_ERR;
>  }
> +static inline void ide_dma_submit_check(IDEState *s,
> +          BlockDriverCompletionFunc *dma_cb, BMDMAState *bm)
> +{
> +    if (bm->aiocb)
> +       return;
> +    dma_cb(bm, -1);
> +}
>  
>  static inline void ide_set_irq(IDEState *s)
>  {
> @@ -933,6 +940,7 @@ static void ide_read_dma_cb(void *opaque, int ret)
>  #endif
>      bm->aiocb = bdrv_aio_read(s->bs, sector_num, s->io_buffer, n,
>                                ide_read_dma_cb, bm);
> +    ide_dma_submit_check(s, ide_read_dma_cb, bm);
>  }
>  
>  static void ide_sector_read_dma(IDEState *s)
> @@ -1035,6 +1043,7 @@ static void ide_write_dma_cb(void *opaque, int ret)
>  #endif
>      bm->aiocb = bdrv_aio_write(s->bs, sector_num, s->io_buffer, n,
>                                 ide_write_dma_cb, bm);
> +    ide_dma_submit_check(s, ide_write_dma_cb, bm);
>  }
>  
>  static void ide_sector_write_dma(IDEState *s)
>