Re: [Qemu-devel] [Qemu-block] [PATCH] block: check BlockDriverState object before dereference

[Stefan Hajnoczi]

On Mon, Jul 17, 2017 at 11:56:48AM -0400, John Snow wrote:
> On 07/11/2017 01:08 PM, P J P wrote:
> > From: Prasad J Pandit <address@hidden>
> > 
> > When processing ATA_CACHE_FLUSH ide controller command,
> > BlockDriverState object could be null. Add check to avoid
> > null pointer dereference.
> > 
> 
> This happens through 0xE7, what QEMU calls WIN_CACHE_FLUSH and described
> in ATA8 ACS3 as "FLUSH CACHE"
> 
> The core of the matter here is that any ATA device associated with a
> BlockBackend that has no media inserted will accept the command and call
> blk_aio_flush, which will later fail because blk_aio_prwv assumes that
> the BlockBackend it was given definitely has a BlockDriverState attached.
> 
> The associated bdrv_inc_in_flight causes the null pointer dereference.
> 
> It's not immediately clear to me what the right fix is here:
> 
> (1) Should blk_ functions not assume they will have a BlockDriverState?
> (i.e. should an attempted blk_flush on an empty blk just succeed
> quietly, or is that inherently an error?)

Hmm...BlockDriverState still has bdrv_is_inserted() even though
BlockBackend->root can be NULL?  CCing Markus in case he has thoughts on
the BB/BDS split.

I find it weird that block-backend.c calls bdrv_inc_in_flight() and then
bdrv_co_flush() will call it again.  Perhaps we need to do this so that
blk_drain() works correctly but it's odd that they share the same
counter variable.

signature.asc
Description: PGP signature