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

[Anthony Liguori]

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