On Wed, Apr 20, 2022 at 10:07:18PM +0200, Andrea Parri (Microsoft) wrote:
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]>
---
include/linux/hyperv.h           |  5 +++++
net/vmw_vsock/hyperv_transport.c | 11 +++++++++--
2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index fe2e0179ed51e..55478a6810b60 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1663,6 +1663,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..092cadc2c866d 100644
--- a/net/vmw_vsock/hyperv_transport.c
+++ b/net/vmw_vsock/hyperv_transport.c
@@ -577,12 +577,19 @@ 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);
+
+       /* Ensure the packet is big enough to read its header */
+       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)
+       /* Ensure the packet is big enough to read its payload */
+       if (payload_len > pkt_len - HVS_HEADER_LEN || payload_len > 
HVS_MTU_SIZE)

checkpatch warns that we exceed 80 characters, I do not have a strong opinion on this, but if you have to resend better break the condition into 2 lines.

Maybe even update or remove the comment? (it only describes the first condition, but the conditions are pretty clear, so I don't think it adds much).

Thanks,
Stefano

_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Reply via email to