[Xen-devel] [PATCH] tools/ocaml: oxenstored: correctly handle a full ring. (Was: Re: [xen-unstable test] 15452: regressions - FAIL)

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2013-02-11 at 11:12 +0000, Ian Campbell wrote:
> 
> 26521:2c0fd406f02c tools/ocaml: oxenstored: Be more paranoid about
> ring reading 

I think this change has broken the case where the ring is full.
Specifically we've gone from

       cons = intf->req_cons;
       prod = intf->req_prod;
       ...
       if (prod == cons)
           return 0;

to:

       cons = intf->req_cons;
       prod = intf->req_prod;
       cons = MASK_XENSTORE_IDX(cons);
       prod = MASK_XENSTORE_IDX(prod);
       ...
       if (prod == cons)
           return 0;

IOW if prod == cons + PAGE_SIZE we now make proc == cons before checking
if they are the same...

8<---------------------------------

>From dd72c63ee56e700b00c0ceffa5d8c150e50fa36f Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@xxxxxxxxxx>
Date: Mon, 11 Feb 2013 11:27:47 +0000
Subject: [PATCH] tools/ocaml: oxenstored: correctly handle a full ring.

Change 26521:2c0fd406f02c (part of XSA-38 / CVE-2013-0215) incorrectly
caused us to ignore rather than process a completely full ring. Check if
producer and consumer are equal before masking to avoid this, since prod ==
cons + PAGE_SIZE after masking becomes prod == cons.

Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
---
 tools/ocaml/libs/xb/xs_ring_stubs.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/ocaml/libs/xb/xs_ring_stubs.c 
b/tools/ocaml/libs/xb/xs_ring_stubs.c
index 4888ac5..fdd9983 100644
--- a/tools/ocaml/libs/xb/xs_ring_stubs.c
+++ b/tools/ocaml/libs/xb/xs_ring_stubs.c
@@ -45,10 +45,10 @@ static int xs_ring_read(struct mmap_interface *interface,
        cons = *(volatile uint32*)&intf->req_cons;
        prod = *(volatile uint32*)&intf->req_prod;
        xen_mb();
-       cons = MASK_XENSTORE_IDX(cons);
-       prod = MASK_XENSTORE_IDX(prod);
        if (prod == cons)
                return 0;
+       cons = MASK_XENSTORE_IDX(cons);
+       prod = MASK_XENSTORE_IDX(prod);
        if (prod > cons)
                to_read = prod - cons;
        else
-- 
1.7.2.5




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel