Re: [PATCH v3 2/3] xen/blkfront: don't take local copy of a request from the ring page
From: Marek Marczykowski-Górecki <hidden>
Date: 2021-09-10 10:14:58
Also in:
lkml, xen-devel
On Fri, Jul 30, 2021 at 12:38:53PM +0200, Juergen Gross wrote:
quoted hunk ↗ jump to hunk
In order to avoid a malicious backend being able to influence the local copy of a request build the request locally first and then copy it to the ring page instead of doing it the other way round as today. Signed-off-by: Juergen Gross <jgross@suse.com> Reviewed-by: Jan Beulich <redacted> Acked-by: Roger Pau Monné <roger.pau@citrix.com> --- V2: - init variable to avoid potential compiler warning (Jan Beulich) --- drivers/block/xen-blkfront.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-)diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c index 15e840287734..b7301006fb28 100644
(...)
quoted hunk ↗ jump to hunk
@@ -827,10 +832,10 @@ static int blkif_queue_rw_req(struct request *req, struct blkfront_ring_info *ri if (setup.segments) kunmap_atomic(setup.segments); - /* Keep a private copy so we can reissue requests when recovering. */ - rinfo->shadow[id].req = *ring_req; + /* Copy request(s) to the ring page. */ + *final_ring_req = *ring_req;
Is this guaranteed to not be optimized by the compiler in an unsafe way (like, do the operation the other way around)? My version of the patch had "wmb()" just before, maybe a good idea to add it here too?
if (unlikely(require_extra_req)) - rinfo->shadow[extra_id].req = *extra_ring_req; + *final_extra_ring_req = *extra_ring_req; if (new_persistent_gnts) gnttab_free_grant_references(setup.gref_head); -- 2.26.2
-- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab
Attachments
- signature.asc [application/pgp-signature] 488 bytes