Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 19 Feb 2019 00:38:16 -0500
From:   Kimberly Brown <kimbrownkd@gmail.com>
To:     Michael Kelley <mikelley@microsoft.com>,
        Long Li <longli@microsoft.com>,
        Sasha Levin <Alexander.Levin@microsoft.com>,
        Stephen Hemminger <sthemmin@microsoft.com>,
        Dexuan Cui <decui@microsoft.com>
Cc:     "K. Y. Srinivasan" <kys@microsoft.com>,
        Haiyang Zhang <haiyangz@microsoft.com>,
        devel@linuxdriverproject.org, linux-kernel@vger.kernel.org
Subject: [PATCH v2 2/2] Drivers: hv: vmbus: Return -EINVAL if
 monitor_allocated not set
Message-ID: <7481d15f52427917a5f620e29308c1aa5c63f3eb.1550554279.git.kimbrownkd@gmail.com>
References: <cover.1549619051.git.kimbrownkd@gmail.com>
 <cover.1550554279.git.kimbrownkd@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <cover.1550554279.git.kimbrownkd@gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

There are two methods for signaling the host: the monitor page mechanism
and hypercalls. The monitor page mechanism is used by performance
critical channels (storage, networking, etc.) because it provides
improved throughput. However, latency is increased. Monitor pages are
allocated to these channels.

Monitor pages are not allocated to channels that do not use the monitor
page mechanism. Therefore, these channels do not have a valid monitor id
or valid monitor page data. In these cases, some of the "_show"
functions return incorrect data. They return an invalid monitor id and
data that is beyond the bounds of the hv_monitor_page array fields.

The "channel->offermsg.monitor_allocated" value can be used to determine
whether monitor pages have been allocated to a channel. In the affected
"_show" functions, verify that "channel->offermsg.monitor_allocated" is
set before accessing the monitor id or the monitor page data. If
"channel->offermsg.monitor_allocated" is not set, return -EINVAL.

Signed-off-by: Kimberly Brown <kimbrownkd@gmail.com>
---
 Documentation/ABI/stable/sysfs-bus-vmbus | 15 ++++++++--
 drivers/hv/vmbus_drv.c                   | 37 ++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/Documentation/ABI/stable/sysfs-bus-vmbus b/Documentation/ABI/stable/sysfs-bus-vmbus
index 3fed8fdb873d..9e8a7a29b3ff 100644
--- a/Documentation/ABI/stable/sysfs-bus-vmbus
+++ b/Documentation/ABI/stable/sysfs-bus-vmbus
@@ -81,7 +81,10 @@ What:		/sys/bus/vmbus/devices/<UUID>/channels/<N>/latency
 Date:		September. 2017
 KernelVersion:	4.14
 Contact:	Stephen Hemminger <sthemmin@microsoft.com>
-Description:	Channel signaling latency
+Description:	Channel signaling latency. The monitor page mechanism is used
+		for performance critical channels (storage, network, etc.).
+		Channels that do not use the monitor page mechanism will return
+		EINVAL.
 Users:		Debugging tools
 
 What:		/sys/bus/vmbus/devices/<UUID>/channels/<N>/out_mask
@@ -95,7 +98,10 @@ What:		/sys/bus/vmbus/devices/<UUID>/channels/<N>/pending
 Date:		September. 2017
 KernelVersion:	4.14
 Contact:	Stephen Hemminger <sthemmin@microsoft.com>
-Description:	Channel interrupt pending state
+Description:	Channel interrupt pending state. The monitor page mechanism is
+		used for performance critical channels (storage, network,
+		etc.). Channels that do not use the monitor page mechanism will
+		return EINVAL.
 Users:		Debugging tools
 
 What:		/sys/bus/vmbus/devices/<UUID>/channels/<N>/read_avail
@@ -137,7 +143,10 @@ What:		/sys/bus/vmbus/devices/<UUID>/channels/<N>/monitor_id
 Date:		January. 2018
 KernelVersion:	4.16
 Contact:	Stephen Hemminger <sthemmin@microsoft.com>
-Description:	Monitor bit associated with channel
+Description:	Monitor bit associated with channel. The monitor page mechanism
+		is used for performance critical channels (storage, network,
+		etc.). Channels that do not use the monitor page mechanism will
+		return EINVAL.
 Users:		Debugging tools and userspace drivers
 
 What:		/sys/bus/vmbus/devices/<UUID>/channels/<N>/ring
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index f2a79f5129d7..0ff9a09a3f71 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -171,6 +171,10 @@ static ssize_t monitor_id_show(struct device *dev,
 
 	if (!hv_dev->channel)
 		return -ENODEV;
+
+	if (!hv_dev->channel->offermsg.monitor_allocated)
+		return -EINVAL;
+
 	return sprintf(buf, "%d\n", hv_dev->channel->offermsg.monitorid);
 }
 static DEVICE_ATTR_RO(monitor_id);
@@ -232,6 +236,10 @@ static ssize_t server_monitor_pending_show(struct device *dev,
 
 	if (!hv_dev->channel)
 		return -ENODEV;
+
+	if (!hv_dev->channel->offermsg.monitor_allocated)
+		return -EINVAL;
+
 	return sprintf(buf, "%d\n",
 		       channel_pending(hv_dev->channel,
 				       vmbus_connection.monitor_pages[0]));
@@ -246,6 +254,10 @@ static ssize_t client_monitor_pending_show(struct device *dev,
 
 	if (!hv_dev->channel)
 		return -ENODEV;
+
+	if (!hv_dev->channel->offermsg.monitor_allocated)
+		return -EINVAL;
+
 	return sprintf(buf, "%d\n",
 		       channel_pending(hv_dev->channel,
 				       vmbus_connection.monitor_pages[1]));
@@ -260,6 +272,10 @@ static ssize_t server_monitor_latency_show(struct device *dev,
 
 	if (!hv_dev->channel)
 		return -ENODEV;
+
+	if (!hv_dev->channel->offermsg.monitor_allocated)
+		return -EINVAL;
+
 	return sprintf(buf, "%d\n",
 		       channel_latency(hv_dev->channel,
 				       vmbus_connection.monitor_pages[0]));
@@ -274,6 +290,10 @@ static ssize_t client_monitor_latency_show(struct device *dev,
 
 	if (!hv_dev->channel)
 		return -ENODEV;
+
+	if (!hv_dev->channel->offermsg.monitor_allocated)
+		return -EINVAL;
+
 	return sprintf(buf, "%d\n",
 		       channel_latency(hv_dev->channel,
 				       vmbus_connection.monitor_pages[1]));
@@ -288,6 +308,10 @@ static ssize_t server_monitor_conn_id_show(struct device *dev,
 
 	if (!hv_dev->channel)
 		return -ENODEV;
+
+	if (!hv_dev->channel->offermsg.monitor_allocated)
+		return -EINVAL;
+
 	return sprintf(buf, "%d\n",
 		       channel_conn_id(hv_dev->channel,
 				       vmbus_connection.monitor_pages[0]));
@@ -302,6 +326,10 @@ static ssize_t client_monitor_conn_id_show(struct device *dev,
 
 	if (!hv_dev->channel)
 		return -ENODEV;
+
+	if (!hv_dev->channel->offermsg.monitor_allocated)
+		return -EINVAL;
+
 	return sprintf(buf, "%d\n",
 		       channel_conn_id(hv_dev->channel,
 				       vmbus_connection.monitor_pages[1]));
@@ -1469,6 +1497,9 @@ static VMBUS_CHAN_ATTR(cpu, S_IRUGO, show_target_cpu, NULL);
 static ssize_t channel_pending_show(const struct vmbus_channel *channel,
 				    char *buf)
 {
+	if (!channel->offermsg.monitor_allocated)
+		return -EINVAL;
+
 	return sprintf(buf, "%d\n",
 		       channel_pending(channel,
 				       vmbus_connection.monitor_pages[1]));
@@ -1478,6 +1509,9 @@ static VMBUS_CHAN_ATTR(pending, S_IRUGO, channel_pending_show, NULL);
 static ssize_t channel_latency_show(const struct vmbus_channel *channel,
 				    char *buf)
 {
+	if (!channel->offermsg.monitor_allocated)
+		return -EINVAL;
+
 	return sprintf(buf, "%d\n",
 		       channel_latency(channel,
 				       vmbus_connection.monitor_pages[1]));
@@ -1499,6 +1533,9 @@ static VMBUS_CHAN_ATTR(events, S_IRUGO, channel_events_show, NULL);
 static ssize_t subchannel_monitor_id_show(const struct vmbus_channel *channel,
 					  char *buf)
 {
+	if (!channel->offermsg.monitor_allocated)
+		return -EINVAL;
+
 	return sprintf(buf, "%u\n", channel->offermsg.monitorid);
 }
 static VMBUS_CHAN_ATTR(monitor_id, S_IRUGO, subchannel_monitor_id_show, NULL);
-- 
2.17.1