| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 14 Feb 2018 10:12:36 -0800
From: Stephen Hemminger <stephen@...workplumber.org>
To: Michael Kelley <mhkelley@...look.com>
Cc: "mikelley@...rosoft.com" <mikelley@...rosoft.com>,
"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"devel@...uxdriverproject.org" <devel@...uxdriverproject.org>,
"olaf@...fle.de" <olaf@...fle.de>,
"apw@...onical.com" <apw@...onical.com>,
"vkuznets@...hat.com" <vkuznets@...hat.com>,
"jasowang@...hat.com" <jasowang@...hat.com>,
"leann.ogasawara@...onical.com" <leann.ogasawara@...onical.com>,
"marcelo.cerri@...onical.com" <marcelo.cerri@...onical.com>,
"sthemmin@...rosoft.com" <sthemmin@...rosoft.com>,
"kys@...rosoft.com" <kys@...rosoft.com>
Subject: Re: [PATCH char-misc 1/1] Drivers: hv: vmbus: Fix ring buffer
signaling
What about the following simplification. If on older hosts you can avoid more code
and the additional barrier. Also cleanup comment wording somewhat.
From de2d566d9092cbbc8e5974dea581617ef787ff69 Mon Sep 17 00:00:00 2001
From: Michael Kelley <mhkelley@...look.com>
Date: Sat, 10 Feb 2018 20:48:49 +0000
Subject: [PATCH 1/9] Drivers: hv: vmbus: Fix ring buffer signaling
Fix bugs in signaling the Hyper-V host when freeing space in the
host->guest ring buffer:
1. The interrupt_mask must not be used to determine whether to signal
on the host->guest ring buffer
2. The ring buffer write_index must be read (via hv_get_bytes_to_write)
*after* pending_send_sz is read in order to avoid a race condition
3. Comparisons with pending_send_sz must treat the "equals" case as
not-enough-space
4. Don't signal if the pending_send_sz feature is not present. Older
versions of Hyper-V that don't implement this feature will poll.
Fixes: 03bad714a161 ("vmbus: more host signalling avoidance")
Signed-off-by: Michael Kelley <mhkelley@...look.com>
---
drivers/hv/ring_buffer.c | 67 +++++++++++++++++++++++++++++-------------------
1 file changed, 40 insertions(+), 27 deletions(-)
diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index 50e071444a5c..d27cbb66279c 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -417,13 +417,25 @@ __hv_pkt_iter_next(struct vmbus_channel *channel,
}
EXPORT_SYMBOL_GPL(__hv_pkt_iter_next);
+/* How many bytes were read in this iterator cycle */
+static u32 hv_pkt_iter_bytes_read(const struct hv_ring_buffer_info *rbi)
+{
+ if (rbi->ring_buffer->read_index < rbi->priv_read_index)
+ return rbi->priv_read_index - rbi->ring_buffer->read_index;
+ else
+ return rbi->ring_datasize -
+ rbi->ring_buffer->read_index + rbi->priv_read_index;
+}
+
/*
* Update host ring buffer after iterating over packets.
*/
void hv_pkt_iter_close(struct vmbus_channel *channel)
{
struct hv_ring_buffer_info *rbi = &channel->inbound;
- u32 orig_write_sz = hv_get_bytes_to_write(rbi);
+ u32 curr_write_sz;
+ u32 delta = hv_pkt_iter_bytes_read(rbi);
+ u32 pending_sz;
/*
* Make sure all reads are done before we update the read index since
@@ -433,39 +445,40 @@ void hv_pkt_iter_close(struct vmbus_channel *channel)
virt_rmb();
rbi->ring_buffer->read_index = rbi->priv_read_index;
+ /* Older versions do not require signalling */
+ if (!rbi->ring_buffer->feature_bits.feat_pending_send_sz)
+ return;
+
/*
- * Issue a full memory barrier before making the signaling decision.
- * Here is the reason for having this barrier:
- * If the reading of the pend_sz (in this function)
- * were to be reordered and read before we commit the new read
- * index (in the calling function) we could
- * have a problem. If the host were to set the pending_sz after we
- * have sampled pending_sz and go to sleep before we commit the
- * read index, we could miss sending the interrupt. Issue a full
- * memory barrier to address this.
+ * If the reading of the pend_sz were to be reordered and read
+ * before we commit the new read index then we would have a
+ * problem. If the host were to set the pending_sz after we
+ * have sampled pending_sz and go to sleep before we commit
+ * the read index, we could miss sending the interrupt.
*/
virt_mb();
- /* If host has disabled notifications then skip */
- if (rbi->ring_buffer->interrupt_mask)
- return;
+ pending_sz = READ_ONCE(rbi->ring_buffer->pending_send_sz);
- if (rbi->ring_buffer->feature_bits.feat_pending_send_sz) {
- u32 pending_sz = READ_ONCE(rbi->ring_buffer->pending_send_sz);
+ /*
+ * Ensure the read of write_index in hv_get_bytes_to_write()
+ * happens after the read of pending_send_sz.
+ */
+ virt_rmb();
+ curr_write_sz = hv_get_bytes_to_write(rbi);
- /*
- * If there was space before we began iteration,
- * then host was not blocked. Also handles case where
- * pending_sz is zero then host has nothing pending
- * and does not need to be signaled.
- */
- if (orig_write_sz > pending_sz)
- return;
+ /*
+ * If there was space before we began iteration,
+ * then host was not blocked. Also handles case where
+ * pending_sz is zero then host has nothing pending and does
+ * not need to be signaled.
+ */
+ if (curr_write_sz - delta > pending_sz)
+ return;
- /* If pending write will not fit, don't give false hope. */
- if (hv_get_bytes_to_write(rbi) < pending_sz)
- return;
- }
+ /* If pending write will not fit, don't give false hope. */
+ if (curr_write_sz <= pending_sz)
+ return;
vmbus_setevent(channel);
}
--
2.15.1
Powered by blists - more mailing lists