[Xen-devel] [PATCH] linux-2.6.18/backends: fix off by one in check of number of requests on the ring

["Jan Beulich" <JBeulich@xxxxxxxx>]

Other than for the "rc" check, where a delta >= RING_SIZE() is bogus,
for "rp" only deltas > RING_SIZE() are, and hence we can't re-use
RING_REQUEST_CONS_OVERFLOW().

While at it, also adjust the values printed to actually be meaningful
should these checks ever trigger: Print the values participating in the
calculation, not an unrelated one.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

--- a/drivers/xen/blkback/blkback.c
+++ b/drivers/xen/blkback/blkback.c
@@ -330,8 +330,8 @@ static int do_block_io_op(blkif_t *blkif
        rp = blk_rings->common.sring->req_prod;
        rmb(); /* Ensure we see queued requests up to 'rp'. */
 
-       /* N.B. 'rp', not 'rc'. */
-       if (RING_REQUEST_CONS_OVERFLOW(&blk_rings->common, rp)) {
+       if (RING_REQUEST_PROD_OVERFLOW(&blk_rings->common, rp)) {
+               rc = blk_rings->common.rsp_prod_pvt;
                printk(KERN_WARNING "blkback:"
                       " Dom%d provided bogus ring requests (%#x - %#x = %u)."
                       " Halting ring processing on dev=%04x\n",
--- a/drivers/xen/blktap/blktap.c
+++ b/drivers/xen/blktap/blktap.c
@@ -1330,8 +1330,8 @@ static int do_block_io_op(blkif_t *blkif
                return 0;
        }
 
-       /* N.B. 'rp', not 'rc'. */
-       if (RING_REQUEST_CONS_OVERFLOW(&blk_rings->common, rp)) {
+       if (RING_REQUEST_PROD_OVERFLOW(&blk_rings->common, rp)) {
+               rc = blk_rings->common.rsp_prod_pvt;
                printk(KERN_WARNING "blktap:"
                       " Dom%d provided bogus ring requests (%#x - %#x = %u)."
                       " Halting ring processing on tap%d\n",
--- a/drivers/xen/scsiback/scsiback.c
+++ b/drivers/xen/scsiback/scsiback.c
@@ -589,8 +589,8 @@ static int scsiback_do_cmd_fn(struct vsc
        rp = ring->sring->req_prod;
        rmb();
 
-       /* N.B. 'rp', not 'rc'. */
-       if (RING_REQUEST_CONS_OVERFLOW(ring, rp)) {
+       if (RING_REQUEST_PROD_OVERFLOW(ring, rp)) {
+               rc = ring->rsp_prod_pvt;
                printk(KERN_WARNING "scsiback:"
                       " Dom%d provided bogus ring requests (%#x - %#x = %u)."
                       " Halting ring processing\n",
--- a/drivers/xen/usbback/usbback.c
+++ b/drivers/xen/usbback/usbback.c
@@ -979,8 +979,8 @@ static int usbbk_start_submit_urb(usbif_
        rp = urb_ring->sring->req_prod;
        rmb();
 
-       /* N.B. 'rp', not 'rc'. */
-       if (RING_REQUEST_CONS_OVERFLOW(urb_ring, rp)) {
+       if (RING_REQUEST_PROD_OVERFLOW(urb_ring, rp)) {
+               rc = urb_ring->rsp_prod_pvt;
                printk(KERN_WARNING "usbback:"
                       " Dom%d provided bogus ring requests (%#x - %#x = %u)."
                       " Halting ring processing on dev=%#x\n",
--- a/include/xen/interface/io/ring.h
+++ b/include/xen/interface/io/ring.h
@@ -234,6 +234,10 @@ typedef struct __name##_back_ring __name
 #define RING_REQUEST_CONS_OVERFLOW(_r, _cons)                           \
     (((_cons) - (_r)->rsp_prod_pvt) >= RING_SIZE(_r))
 
+/* Ill-behaved frontend determination: Can there be this many requests? */
+#define RING_REQUEST_PROD_OVERFLOW(_r, _prod)                           \
+    (((_prod) - (_r)->rsp_prod_pvt) > RING_SIZE(_r))
+
 #define RING_PUSH_REQUESTS(_r) do {                                     \
     xen_wmb(); /* back sees requests /before/ updated producer index */ \
     (_r)->sring->req_prod = (_r)->req_prod_pvt;                         \

Attachment: xen-backends-reject-ring-overruns-fix.patch
Description: Text document

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