From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: [PATCH] linux-2.6.18/usbback: don't access request fields in shared ring more than once Date: Wed, 01 Oct 2014 07:29:18 +0100 Message-ID: <542BBB5E020000780003B500@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=__PartA693D42E.0__=" Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XZDPd-0004dA-83 for xen-devel@lists.xenproject.org; Wed, 01 Oct 2014 06:29:21 +0000 List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: xen-devel Cc: Juergen Gross List-Id: xen-devel@lists.xenproject.org This is a MIME message. If you are reading this text, you may want to consider changing to a mail reader or gateway that understands how to properly handle MIME multipart messages. --=__PartA693D42E.0__= Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Content-Disposition: inline ... 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 --- 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 =3D &usbif->urb_ring; - usbif_urb_request_t *req; pending_req_t *pending_req; RING_IDX rc, rp; int more_to_do =3D 0; @@ -989,6 +988,8 @@ static int usbbk_start_submit_urb(usbif_ } =20 while (rc !=3D 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; } =20 - req =3D RING_GET_REQUEST(urb_ring, rc); - - dispatch_request_to_pending_reqs(usbif, req, - pending_req); + req =3D *RING_GET_REQUEST(urb_ring, rc); urb_ring->req_cons =3D ++rc; + barrier(); + + dispatch_request_to_pending_reqs(usbif, &req, pending_req);= =20 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 =3D &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 =20 spin_lock_irqsave(&usbif->conn_ring_lock, flags); =20 - req =3D RING_GET_REQUEST(ring, ring->req_cons);; - id =3D req->id; + id =3D RING_GET_REQUEST(ring, ring->req_cons)->id; ring->req_cons++; ring->sring->req_event =3D ring->req_cons + 1; =20 --=__PartA693D42E.0__= Content-Type: text/plain; name="xen-usbback-copy-req.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="xen-usbback-copy-req.patch" usbback: don't access request fields in shared ring more than once=0A=0A...= as being unsafe (prone to the fields changing under our feet). Now=0Athat = we copy the request, undo 1264:ab22257ffed2 ("usbback: mark=0Arequest as = consumed after finished using it") at once.=0A=0AFor the usbbk_hotplug_noti= fy() case adjust the code slightly too=0Adespite there not being explicit = issues - just make obvious that there=0Aare no multiple field accesses = here by not even using an intermediate=0Avariable.=0A=0ASigned-off-by: Jan = Beulich =0A=0A--- a/drivers/xen/usbback/usbback.c=0A+++ = b/drivers/xen/usbback/usbback.c=0A@@ -970,7 +970,6 @@ fail_response:=0A = static int usbbk_start_submit_urb(usbif_t *usbif)=0A {=0A usbif_urb_b= ack_ring_t *urb_ring =3D &usbif->urb_ring;=0A- usbif_urb_request_t = *req;=0A pending_req_t *pending_req;=0A RING_IDX rc, rp;=0A = int more_to_do =3D 0;=0A@@ -989,6 +988,8 @@ static int usbbk_start_submit_u= rb(usbif_=0A }=0A =0A while (rc !=3D rp) {=0A+ = usbif_urb_request_t req;=0A+=0A if (RING_REQUEST_CONS_OVERF= LOW(urb_ring, rc)) {=0A if(printk_ratelimit())=0A = printk(KERN_WARNING "usbback: "=0A@@ -1002,11 = +1003,11 @@ static int usbbk_start_submit_urb(usbif_=0A = break;=0A }=0A =0A- req =3D RING_GET_REQUEST(ur= b_ring, rc);=0A-=0A- dispatch_request_to_pending_reqs(usbif, = req,=0A- pending_req= );=0A+ req =3D *RING_GET_REQUEST(urb_ring, rc);=0A = urb_ring->req_cons =3D ++rc;=0A+ barrier();=0A+=0A+ = dispatch_request_to_pending_reqs(usbif, &req, pending_req);=0A =0A = cond_resched();=0A }=0A@@ -1019,7 +1020,6 @@ static int usbbk_start_su= bmit_urb(usbif_=0A void usbbk_hotplug_notify(usbif_t *usbif, int portnum, = int speed)=0A {=0A usbif_conn_back_ring_t *ring =3D &usbif->conn_ring;= =0A- usbif_conn_request_t *req;=0A usbif_conn_response_t *res;=0A = unsigned long flags;=0A u16 id;=0A@@ -1027,8 +1027,7 @@ void = usbbk_hotplug_notify(usbif_t *usbif=0A =0A spin_lock_irqsave(&usbif->c= onn_ring_lock, flags);=0A =0A- req =3D RING_GET_REQUEST(ring, ring->req_co= ns);;=0A- id =3D req->id;=0A+ id =3D RING_GET_REQUEST(ring, = ring->req_cons)->id;=0A ring->req_cons++;=0A ring->sring->req_ev= ent =3D ring->req_cons + 1;=0A =0A --=__PartA693D42E.0__= Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --=__PartA693D42E.0__=--