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 <p...@fedoraproject.org> > > > > 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
