From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52398) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1apyqc-0001Iy-Ga for qemu-devel@nongnu.org; Tue, 12 Apr 2016 09:59:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1apyqZ-0005Q4-AK for qemu-devel@nongnu.org; Tue, 12 Apr 2016 09:59:18 -0400 Received: from smtp02.citrix.com ([66.165.176.63]:59553) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1apyqZ-0005Pu-4w for qemu-devel@nongnu.org; Tue, 12 Apr 2016 09:59:15 -0400 References: <1460457796-1779-1-git-send-email-wei.liu2@citrix.com> <1460457796-1779-3-git-send-email-wei.liu2@citrix.com> <570CF0B1.9070507@citrix.com> From: Andrew Cooper Message-ID: <570CFA45.7070504@citrix.com> Date: Tue, 12 Apr 2016 14:38:13 +0100 MIME-Version: 1.0 In-Reply-To: <570CF0B1.9070507@citrix.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Xen-devel] [PATCH v2 2/3] xenfb: move xen_rmb to the correct location List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Vrabel , Wei Liu , qemu-devel@nongnu.org Cc: Anthony Perard , Xen-devel , Stefano Stabellini On 12/04/16 13:57, David Vrabel wrote: > On 12/04/16 11:43, Wei Liu wrote: >> It should be placed before first time producer and consumer are used. > This change isn't necessary and is confusing as this is not what this > barrier is for. > > The barrier needs to be between the load of prod and the load of the > ring contents (there's even a comment that says this). This pairs with > the corresponding write barrier between the store of the ring contents > and the store of prod (in the other end). Looking further, this code will compile to multiple reads of the page, because there is no ACCESS_ONCE(). This code is still vulnerable to XSA-155. ~Andrew > > David > >> Signed-off-by: Wei Liu >> --- >> Cc: Stefano Stabellini >> Cc: Anthony Perard >> >> Backport candidate to our own tree. >> --- >> hw/display/xenfb.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c >> index 9866dfd..7f4fad7 100644 >> --- a/hw/display/xenfb.c >> +++ b/hw/display/xenfb.c >> @@ -775,10 +775,10 @@ static void xenfb_handle_events(struct XenFB *xenfb) >> >> prod = page->out_prod; >> out_cons = page->out_cons; >> + xen_rmb(); >> if (prod - out_cons > XENFB_OUT_RING_LEN) { >> return; >> } >> - xen_rmb(); /* ensure we see ring contents up to prod */ >> for (cons = out_cons; cons != prod; cons++) { >> union xenfb_out_event *event = &XENFB_OUT_RING_REF(page, cons); >> uint8_t type = event->type; >> > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel