[PATCH] linux-2.6.18/usbback: don't access request fields in shared ring more than once

[PATCH] linux-2.6.18/usbback: don't access request fields in shared ring more than once

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 2027 bytes --]

... as being unsafe (prone to the fields changing under our feet). Now
that we copy the request, undo 1264:ab22257ffed2 ("usbback: mark
request as consumed after finished using it") at once.

For the usbbk_hotplug_notify() case adjust the code slightly too
despite there not being explicit issues - just make obvious that there
are no multiple field accesses here by not even using an intermediate
variable.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/drivers/xen/usbback/usbback.c
+++ b/drivers/xen/usbback/usbback.c
@@ -970,7 +970,6 @@ fail_response:
 static int usbbk_start_submit_urb(usbif_t *usbif)
 {
 	usbif_urb_back_ring_t *urb_ring = &usbif->urb_ring;
-	usbif_urb_request_t *req;
 	pending_req_t *pending_req;
 	RING_IDX rc, rp;
 	int more_to_do = 0;
@@ -989,6 +988,8 @@ static int usbbk_start_submit_urb(usbif_
 	}
 
 	while (rc != rp) {
+		usbif_urb_request_t req;
+
 		if (RING_REQUEST_CONS_OVERFLOW(urb_ring, rc)) {
 			if(printk_ratelimit())
 				printk(KERN_WARNING "usbback: "
@@ -1002,11 +1003,11 @@ static int usbbk_start_submit_urb(usbif_
 			break;
 		}
 
-		req = RING_GET_REQUEST(urb_ring, rc);
-
-		dispatch_request_to_pending_reqs(usbif, req,
-							pending_req);
+		req = *RING_GET_REQUEST(urb_ring, rc);
 		urb_ring->req_cons = ++rc;
+		barrier();
+
+		dispatch_request_to_pending_reqs(usbif, &req, pending_req);
 
 		cond_resched();
 	}
@@ -1019,7 +1020,6 @@ static int usbbk_start_submit_urb(usbif_
 void usbbk_hotplug_notify(usbif_t *usbif, int portnum, int speed)
 {
 	usbif_conn_back_ring_t *ring = &usbif->conn_ring;
-	usbif_conn_request_t *req;
 	usbif_conn_response_t *res;
 	unsigned long flags;
 	u16 id;
@@ -1027,8 +1027,7 @@ void usbbk_hotplug_notify(usbif_t *usbif
 
 	spin_lock_irqsave(&usbif->conn_ring_lock, flags);
 
-	req = RING_GET_REQUEST(ring, ring->req_cons);;
-	id = req->id;
+	id = RING_GET_REQUEST(ring, ring->req_cons)->id;
 	ring->req_cons++;
 	ring->sring->req_event = ring->req_cons + 1;
 




[-- Attachment #2: xen-usbback-copy-req.patch --]
[-- Type: text/plain, Size: 2091 bytes --]

usbback: don't access request fields in shared ring more than once

... as being unsafe (prone to the fields changing under our feet). Now
that we copy the request, undo 1264:ab22257ffed2 ("usbback: mark
request as consumed after finished using it") at once.

For the usbbk_hotplug_notify() case adjust the code slightly too
despite there not being explicit issues - just make obvious that there
are no multiple field accesses here by not even using an intermediate
variable.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/drivers/xen/usbback/usbback.c
+++ b/drivers/xen/usbback/usbback.c
@@ -970,7 +970,6 @@ fail_response:
 static int usbbk_start_submit_urb(usbif_t *usbif)
 {
 	usbif_urb_back_ring_t *urb_ring = &usbif->urb_ring;
-	usbif_urb_request_t *req;
 	pending_req_t *pending_req;
 	RING_IDX rc, rp;
 	int more_to_do = 0;
@@ -989,6 +988,8 @@ static int usbbk_start_submit_urb(usbif_
 	}
 
 	while (rc != rp) {
+		usbif_urb_request_t req;
+
 		if (RING_REQUEST_CONS_OVERFLOW(urb_ring, rc)) {
 			if(printk_ratelimit())
 				printk(KERN_WARNING "usbback: "
@@ -1002,11 +1003,11 @@ static int usbbk_start_submit_urb(usbif_
 			break;
 		}
 
-		req = RING_GET_REQUEST(urb_ring, rc);
-
-		dispatch_request_to_pending_reqs(usbif, req,
-							pending_req);
+		req = *RING_GET_REQUEST(urb_ring, rc);
 		urb_ring->req_cons = ++rc;
+		barrier();
+
+		dispatch_request_to_pending_reqs(usbif, &req, pending_req);
 
 		cond_resched();
 	}
@@ -1019,7 +1020,6 @@ static int usbbk_start_submit_urb(usbif_
 void usbbk_hotplug_notify(usbif_t *usbif, int portnum, int speed)
 {
 	usbif_conn_back_ring_t *ring = &usbif->conn_ring;
-	usbif_conn_request_t *req;
 	usbif_conn_response_t *res;
 	unsigned long flags;
 	u16 id;
@@ -1027,8 +1027,7 @@ void usbbk_hotplug_notify(usbif_t *usbif
 
 	spin_lock_irqsave(&usbif->conn_ring_lock, flags);
 
-	req = RING_GET_REQUEST(ring, ring->req_cons);;
-	id = req->id;
+	id = RING_GET_REQUEST(ring, ring->req_cons)->id;
 	ring->req_cons++;
 	ring->sring->req_event = ring->req_cons + 1;
 

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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