From: John Snow <jsnow@redhat.com>
To: Simon Rowe <simon.rowe@nutanix.com>
Cc: Fiona Ebner <f.ebner@proxmox.com>,
Niklas Cassel <niklas.cassel@wdc.com>,
qemu-devel <qemu-devel@nongnu.org>,
Qemu-block <qemu-block@nongnu.org>,
Felipe Franciosi <felipe@nutanix.com>
Subject: Re: [PATCH 1/1] hw/ide/core: terminate in-flight DMA on IDE bus reset
Date: Mon, 2 Oct 2023 18:59:17 -0400 [thread overview]
Message-ID: <CAFn=p-a7UsM=dmJjmLpKmd1WYgNnfxQR1AsOQtCR64uZwVYZeA@mail.gmail.com> (raw)
In-Reply-To: <DM8PR02MB81217871CAB18CCBB04938F893C5A@DM8PR02MB8121.namprd02.prod.outlook.com>
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
next prev parent reply other threads:[~2023-10-02 23:00 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-21 16:07 [PATCH 0/1] CVE-2023-5088 Simon Rowe
2023-09-21 16:07 ` [PATCH 1/1] hw/ide/core: terminate in-flight DMA on IDE bus reset Simon Rowe
2023-09-25 19:53 ` John Snow
2023-09-26 7:11 ` Fiona Ebner
2023-09-26 14:45 ` John Snow
2023-09-28 11:23 ` Fiona Ebner
2023-10-02 9:08 ` Simon Rowe
2023-10-02 22:59 ` John Snow [this message]
2023-10-03 12:46 ` Simon Rowe
2023-10-03 14:06 ` Niklas Cassel
2023-10-03 17:05 ` John Snow
2023-10-04 7:51 ` Simon Rowe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAFn=p-a7UsM=dmJjmLpKmd1WYgNnfxQR1AsOQtCR64uZwVYZeA@mail.gmail.com' \
--to=jsnow@redhat.com \
--cc=f.ebner@proxmox.com \
--cc=felipe@nutanix.com \
--cc=niklas.cassel@wdc.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=simon.rowe@nutanix.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).