From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1KcAL4-0003xj-6z
	for qemu-devel@nongnu.org; Sat, 06 Sep 2008 22:49:22 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1KcAL2-0003xP-Q7
	for qemu-devel@nongnu.org; Sat, 06 Sep 2008 22:49:20 -0400
Received: from [199.232.76.173] (port=36057 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1KcAL2-0003xM-J8
	for qemu-devel@nongnu.org; Sat, 06 Sep 2008 22:49:20 -0400
Received: from an-out-0708.google.com ([209.85.132.243]:59851)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <anthony@codemonkey.ws>) id 1KcAL2-0003PT-5X
	for qemu-devel@nongnu.org; Sat, 06 Sep 2008 22:49:20 -0400
Received: by an-out-0708.google.com with SMTP id d18so163387and.130
	for <qemu-devel@nongnu.org>; Sat, 06 Sep 2008 19:49:19 -0700 (PDT)
Message-ID: <48C340FF.3000503@codemonkey.ws>
Date: Sat, 06 Sep 2008 21:48:31 -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>	<48B70949.5040906@codemonkey.ws>
	<18615.50224.233594.405241@mariner.uk.xensource.com>
In-Reply-To: <18615.50224.233594.405241@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:
> Anthony Liguori writes ("Re: [Qemu-devel] [PATCH] [ide] Check that asynchronous (DMA) submission succeeds"):
>   
>> 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.
>>     
>
> I'm not sure exactly what you mean but I'm sure that this is not a
> problem in this case.  In my patch ide_dma_submit_check is called from
> two places:
>  * ide_read_dma_cb, with ide_read_dma_cb as the callback argument
>  * ide_write_dma_cb, with ide_write_dma_cb as the callback argument
>
> In both places the only situation where the callback is reentered
> immediately is if the aio submission failed.  So in that case we
> recursively enter the callback function, and we do so exactly once
> since we're going to execute the error handling case (ret==-1).
>
> Note that the call to ide_dma_submit_check is at the end of
> ide_{read,write}_dma_cb precisely to avoid any kind of reentrancy
> problem.
>   

I'll have to look more closely, but most of the code goes to great 
lengths to use bottom halves to avoid the possibility of infinite 
recursion.  The concern with recursion is not CPU consumption, it's that 
you'll eventually overrun the stack and potentially crash the QEMU process.

You may be right that in this case, recursion is impossible but it's 
probably better to use a bottom half just for the sake of consistency.

Regards,

Anthony Liguori