Re: [PATCH 1/1] hw/ide/core: terminate in-flight DMA on IDE bus reset

[John Snow]

On Mon, Oct 2, 2023 at 5:09 AM Simon Rowe <simon.rowe@nutanix.com> wrote:
>
> On Thursday, 28 September 2023 Fiona Ebner <f.ebner@proxmox.com> wrote:
>
>
>
> > AFAICT, yes, because the DMA callback is invoked before resetting the
> > state now. But not 100% sure if it can't be triggered in some other way,
> > maybe Simon knows more? I don't have a reproducer for the CVE either,
> > but the second patch after the one linked above adds a qtest for the
> > reset scenario.
>
>
>
> I initially tested an identical change and, yes, it did seem to address the 
> issue. I preferred my final solution because it felt wrong for the DMA to 
> continue after the point the VM is expecting the controller to be reset. I 
> felt it was best to leave the ordering as is (because there are multiple 
> other controllers that use ide_bus_reset()) and terminate the DMA transaction 
> using state that indicates a reset is being performed.

Which reset pathway are you testing that causes the problem? I'm not
fully clear on why checking for DRQ is legitimate here. Does this new
condition fire before or after the registers have been reset as part
of the reset ...?

I trust you there's a problem, but I don't know the specifics of it
just yet to understand your proposed fix. (I have a nagging fear that
it might trigger in more circumstances than we want it to, but I need
more info to audit that.)

I'm also concerned about -- depending on WHEN this conditional will
fire -- the effects of processing the end-of-transfer block on a newly
reset (or about-to-be reset) device.

>
>
>
> I have a test setup that I use to reproduce this (that was mentioned in the 
> original CVE disclosure). My patch ran for 24+ hours successfully. I can test 
> any other proposed fix.
>
>
>
> Regards
>
> Simon