diff mbox series

UBUNTU: SAUCE: hv/bounce buffer: Fix a race that can fail disk detection

Message ID 20220502150218.12340-2-tim.gardner@canonical.com
State New
Headers show
Series hv/bounce buffer: Fix a race that can fail disk detection | expand

Commit Message

Tim Gardner May 2, 2022, 3:02 p.m. UTC 
From: Dexuan Cui <decui@microsoft.com>

BugLink: https://bugs.launchpad.net/bugs/1971164

As soon as hv_ringbuffer_write() returns, the host's response can arrive
immediately, and the channel callback of hv_storvsc can be called -- at
this time, if the "*pbounce_pkt" is still not assigned, the channel
callback is unable to find the bounce_pkt info, and consequently
storvsc_on_channel_callback() -> hv_pkt_bounce() is not called,
i.e. the bounce buffer is leaked and we fail to copy the bounce buffer
to the private SCSI response buffer, and from the VM's perspective it
looks like the host didn't write the response, and the VM's SCSI commands
(e.g. INQUERY, READ) may fail unexpectedly, and sometimes some of the
disks can not be detected properly.

Fix the race by moving the *pbounce_pkt line to before
hv_ringbuffer_write(), and resetting it if hv_ringbuffer_write() fails.

Fixes: df6c139e3297 ("UBUNTU: SAUCE: x86/Hyper-V: Copy data from/to bounce buffer during IO operation.")
Signed-off-by: Dexuan Cui <decui@microsoft.com>
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
 drivers/hv/hv_bounce.c | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)
diff mbox series

Patch

diff --git a/drivers/hv/hv_bounce.c b/drivers/hv/hv_bounce.c
index 69741e97f414e..1679014e996cc 100644
--- a/drivers/hv/hv_bounce.c
+++ b/drivers/hv/hv_bounce.c
@@ -578,11 +578,21 @@  int vmbus_sendpacket_pagebuffer_bounce(
 			(struct hv_page_range *)desc->range, io_type);
 	if (unlikely(!bounce_pkt))
 		return -EAGAIN;
+
+	/*
+	 * This assignment must be before hv_ringbuffer_write(), because as
+	 * soon as hv_ringbuffer_write() returns, the channel callback may
+	 * be running, and the callback needs request->bounce_pkt, which is
+	 * assigned in this function. Note: if hv_ringbuffer_write() fails,
+	 * *pbounce_pkt must be reset to NULL.
+	 */
+	*pbounce_pkt = bounce_pkt;
+
 	ret = hv_ringbuffer_write(channel, bufferlist, 3, requestid);
-	if (unlikely(ret < 0))
+	if (unlikely(ret < 0)) {
 		hv_bounce_resources_release(channel, bounce_pkt);
-	else
-		*pbounce_pkt = bounce_pkt;
+		*pbounce_pkt = NULL;
+	}
 
 	return ret;
 }
@@ -619,13 +629,23 @@  int vmbus_sendpacket_mpb_desc_bounce(
 	if (unlikely(!bounce_pkt))
 		goto free;
 	bufferlist[0].iov_base = desc_bounce;
+
+	/*
+	 * This assignment must be before hv_ringbuffer_write(), because as
+	 * soon as hv_ringbuffer_write() returns, the channel callback may
+	 * be running, and the callback needs request->bounce_pkt, which is
+	 * assigned in this function. Note: if hv_ringbuffer_write() fails,
+	 * *pbounce_pkt must be reset to NULL.
+	 */
+	*pbounce_pkt = bounce_pkt;
+
 	ret = hv_ringbuffer_write(channel, bufferlist, 3, requestid);
 free:
 	kfree(desc_bounce);
-	if (unlikely(ret < 0))
+	if (unlikely(ret < 0)) {
 		hv_bounce_resources_release(channel, bounce_pkt);
-	else
-		*pbounce_pkt = bounce_pkt;
+		*pbounce_pkt = NULL;
+	}
 	return ret;
 }