From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1KYo2e-0000bZ-EO
	for qemu-devel@nongnu.org; Thu, 28 Aug 2008 16:24:28 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1KYo2c-0000aq-Sz
	for qemu-devel@nongnu.org; Thu, 28 Aug 2008 16:24:28 -0400
Received: from [199.232.76.173] (port=39521 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1KYo2c-0000an-PV
	for qemu-devel@nongnu.org; Thu, 28 Aug 2008 16:24:26 -0400
Received: from an-out-0708.google.com ([209.85.132.246]:19307)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <anthony@codemonkey.ws>) id 1KYo2c-0008QB-6T
	for qemu-devel@nongnu.org; Thu, 28 Aug 2008 16:24:26 -0400
Received: by an-out-0708.google.com with SMTP id d18so87933and.130
	for <qemu-devel@nongnu.org>; Thu, 28 Aug 2008 13:24:24 -0700 (PDT)
Message-ID: <48B70949.5040906@codemonkey.ws>
Date: Thu, 28 Aug 2008 15:23:37 -0500
From: Anthony Liguori <anthony@codemonkey.ws>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [PATCH] [ide] Check that asynchronous (DMA)
	submission succeeds
References: <18614.55228.190438.575778@mariner.uk.xensource.com>
In-Reply-To: <18614.55228.190438.575778@mariner.uk.xensource.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Reply-To: qemu-devel@nongnu.org
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: qemu-devel@nongnu.org

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 <ian.jackson@eu.citrix.com>
> ---
>  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)
>