On Mon, Oct 2, 2023 at 5:09 AM Simon Rowe <simon.r...@nutanix.com> wrote: > > On Thursday, 28 September 2023 Fiona Ebner <f.eb...@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
