Changes since v1[1]:
- Expand commit message of patch #2
- Coding style changes
Changes since RFC[2]:
- Massage changelogs, fix typo
- Drop "hv_sock: Initialize send_buf in hvs_stream_enqueue()"
- Remove style/newline change
- Remove/"inline" hv_pkt_iter_first_raw()
Thanks,
Andrea
[1] https://lkml.kernel.org/r/[email protected]
[2] https://lkml.kernel.org/r/[email protected]
Andrea Parri (Microsoft) (5):
hv_sock: Check hv_pkt_iter_first_raw()'s return value
hv_sock: Copy packets sent by Hyper-V out of the ring buffer
hv_sock: Add validation for untrusted Hyper-V values
Drivers: hv: vmbus: Accept hv_sock offers in isolated guests
Drivers: hv: vmbus: Refactor the ring-buffer iterator functions
drivers/hv/channel_mgmt.c | 8 ++++--
drivers/hv/ring_buffer.c | 32 ++++++---------------
include/linux/hyperv.h | 48 ++++++++++----------------------
net/vmw_vsock/hyperv_transport.c | 21 +++++++++++---
4 files changed, 47 insertions(+), 62 deletions(-)
--
2.25.1
Pointers to VMbus packets sent by Hyper-V are used by the hv_sock driver
within the guest VM. Hyper-V can send packets with erroneous values or
modify packet fields after they are processed by the guest. To defend
against these scenarios, copy the incoming packet after validating its
length and offset fields using hv_pkt_iter_{first,next}(). Use
HVS_PKT_LEN(HVS_MTU_SIZE) to initialize the buffer which holds the
copies of the incoming packets. In this way, the packet can no longer
be modified by the host.
Signed-off-by: Andrea Parri (Microsoft) <[email protected]>
Reviewed-by: Michael Kelley <[email protected]>
Reviewed-by: Stefano Garzarella <[email protected]>
---
net/vmw_vsock/hyperv_transport.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
index 943352530936e..8c37d07017fc4 100644
--- a/net/vmw_vsock/hyperv_transport.c
+++ b/net/vmw_vsock/hyperv_transport.c
@@ -78,6 +78,9 @@ struct hvs_send_buf {
ALIGN((payload_len), 8) + \
VMBUS_PKT_TRAILER_SIZE)
+/* Upper bound on the size of a VMbus packet for hv_sock */
+#define HVS_MAX_PKT_SIZE HVS_PKT_LEN(HVS_MTU_SIZE)
+
union hvs_service_id {
guid_t srv_id;
@@ -378,6 +381,8 @@ static void hvs_open_connection(struct vmbus_channel *chan)
rcvbuf = ALIGN(rcvbuf, HV_HYP_PAGE_SIZE);
}
+ chan->max_pkt_size = HVS_MAX_PKT_SIZE;
+
ret = vmbus_open(chan, sndbuf, rcvbuf, NULL, 0, hvs_channel_cb,
conn_from_host ? new : sk);
if (ret != 0) {
@@ -602,7 +607,7 @@ static ssize_t hvs_stream_dequeue(struct vsock_sock *vsk, struct msghdr *msg,
return -EOPNOTSUPP;
if (need_refill) {
- hvs->recv_desc = hv_pkt_iter_first_raw(hvs->chan);
+ hvs->recv_desc = hv_pkt_iter_first(hvs->chan);
if (!hvs->recv_desc)
return -ENOBUFS;
ret = hvs_update_recv_data(hvs);
@@ -618,7 +623,7 @@ static ssize_t hvs_stream_dequeue(struct vsock_sock *vsk, struct msghdr *msg,
hvs->recv_data_len -= to_read;
if (hvs->recv_data_len == 0) {
- hvs->recv_desc = hv_pkt_iter_next_raw(hvs->chan, hvs->recv_desc);
+ hvs->recv_desc = hv_pkt_iter_next(hvs->chan, hvs->recv_desc);
if (hvs->recv_desc) {
ret = hvs_update_recv_data(hvs);
if (ret)
--
2.25.1
For additional robustness in the face of Hyper-V errors or malicious
behavior, validate all values that originate from packets that Hyper-V
has sent to the guest in the host-to-guest ring buffer. Ensure that
invalid values cannot cause data being copied out of the bounds of the
source buffer in hvs_stream_dequeue().
Signed-off-by: Andrea Parri (Microsoft) <[email protected]>
Reviewed-by: Michael Kelley <[email protected]>
Reviewed-by: Stefano Garzarella <[email protected]>
---
include/linux/hyperv.h | 5 +++++
net/vmw_vsock/hyperv_transport.c | 10 ++++++++--
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 460a716f47486..a01c9fd0a3348 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1696,6 +1696,11 @@ static inline u32 hv_pkt_datalen(const struct vmpacket_descriptor *desc)
return (desc->len8 << 3) - (desc->offset8 << 3);
}
+/* Get packet length associated with descriptor */
+static inline u32 hv_pkt_len(const struct vmpacket_descriptor *desc)
+{
+ return desc->len8 << 3;
+}
struct vmpacket_descriptor *
hv_pkt_iter_first_raw(struct vmbus_channel *channel);
diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
index 8c37d07017fc4..fd98229e3db30 100644
--- a/net/vmw_vsock/hyperv_transport.c
+++ b/net/vmw_vsock/hyperv_transport.c
@@ -577,12 +577,18 @@ static bool hvs_dgram_allow(u32 cid, u32 port)
static int hvs_update_recv_data(struct hvsock *hvs)
{
struct hvs_recv_buf *recv_buf;
- u32 payload_len;
+ u32 pkt_len, payload_len;
+
+ pkt_len = hv_pkt_len(hvs->recv_desc);
+
+ if (pkt_len < HVS_HEADER_LEN)
+ return -EIO;
recv_buf = (struct hvs_recv_buf *)(hvs->recv_desc + 1);
payload_len = recv_buf->hdr.data_size;
- if (payload_len > HVS_MTU_SIZE)
+ if (payload_len > pkt_len - HVS_HEADER_LEN ||
+ payload_len > HVS_MTU_SIZE)
return -EIO;
if (payload_len == 0)
--
2.25.1
On Thu, Apr 28, 2022 at 04:51:02PM +0200, Andrea Parri (Microsoft) wrote:
>
> Andrea Parri (Microsoft) (5):
> hv_sock: Check hv_pkt_iter_first_raw()'s return value
> hv_sock: Copy packets sent by Hyper-V out of the ring buffer
> hv_sock: Add validation for untrusted Hyper-V values
> Drivers: hv: vmbus: Accept hv_sock offers in isolated guests
> Drivers: hv: vmbus: Refactor the ring-buffer iterator functions
Applied to hyperv-next. Thanks.