Drivers: hv: check VMBus messages lengths

VMBus message handlers (channel_message_table) receive a pointer to
'struct vmbus_channel_message_header' and cast it to a structure of their
choice, which is sometimes longer than the header. We, however, don't check
that the message is long enough so in case hypervisor screws up we'll be
accessing memory beyond what was allocated for temporary buffer.

Previously, we used to always allocate and copy 256 bytes from message page
to temporary buffer but this is hardly better: in case the message is
shorter than we expect we'll be trying to consume garbage as some real
data and no memory guarding technique will be able to identify an issue.

Introduce 'min_payload_len' to 'struct vmbus_channel_message_table_entry'
and check against it in vmbus_on_msg_dpc(). Note, we can't require the
exact length as new hypervisor versions may add extra fields to messages,
we only check that the message is not shorter than we expect.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
Link: https://lore.kernel.org/r/20200406104326.45361-1-vkuznets@redhat.com
Signed-off-by: Wei Liu <wei.liu@kernel.org>

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index da93b5f1b143667f0097171a4f4c3d719d5d3aa4..3a5700c8486afe0ca48509d4daf25ebd1a3779af 100644 (file)
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -1332,30 +1332,36 @@ static void vmbus_onversion_response(
 /* Channel message dispatch table */
 const struct vmbus_channel_message_table_entry
 channel_message_table[CHANNELMSG_COUNT] = {
-       { CHANNELMSG_INVALID,                   0, NULL },
-       { CHANNELMSG_OFFERCHANNEL,              0, vmbus_onoffer },
-       { CHANNELMSG_RESCIND_CHANNELOFFER,      0, vmbus_onoffer_rescind },
-       { CHANNELMSG_REQUESTOFFERS,             0, NULL },
-       { CHANNELMSG_ALLOFFERS_DELIVERED,       1, vmbus_onoffers_delivered },
-       { CHANNELMSG_OPENCHANNEL,               0, NULL },
-       { CHANNELMSG_OPENCHANNEL_RESULT,        1, vmbus_onopen_result },
-       { CHANNELMSG_CLOSECHANNEL,              0, NULL },
-       { CHANNELMSG_GPADL_HEADER,              0, NULL },
-       { CHANNELMSG_GPADL_BODY,                0, NULL },
-       { CHANNELMSG_GPADL_CREATED,             1, vmbus_ongpadl_created },
-       { CHANNELMSG_GPADL_TEARDOWN,            0, NULL },
-       { CHANNELMSG_GPADL_TORNDOWN,            1, vmbus_ongpadl_torndown },
-       { CHANNELMSG_RELID_RELEASED,            0, NULL },
-       { CHANNELMSG_INITIATE_CONTACT,          0, NULL },
-       { CHANNELMSG_VERSION_RESPONSE,          1, vmbus_onversion_response },
-       { CHANNELMSG_UNLOAD,                    0, NULL },
-       { CHANNELMSG_UNLOAD_RESPONSE,           1, vmbus_unload_response },
-       { CHANNELMSG_18,                        0, NULL },
-       { CHANNELMSG_19,                        0, NULL },
-       { CHANNELMSG_20,                        0, NULL },
-       { CHANNELMSG_TL_CONNECT_REQUEST,        0, NULL },
-       { CHANNELMSG_22,                        0, NULL },
-       { CHANNELMSG_TL_CONNECT_RESULT,         0, NULL },
+       { CHANNELMSG_INVALID,                   0, NULL, 0},
+       { CHANNELMSG_OFFERCHANNEL,              0, vmbus_onoffer,
+               sizeof(struct vmbus_channel_offer_channel)},
+       { CHANNELMSG_RESCIND_CHANNELOFFER,      0, vmbus_onoffer_rescind,
+               sizeof(struct vmbus_channel_rescind_offer) },
+       { CHANNELMSG_REQUESTOFFERS,             0, NULL, 0},
+       { CHANNELMSG_ALLOFFERS_DELIVERED,       1, vmbus_onoffers_delivered, 0},
+       { CHANNELMSG_OPENCHANNEL,               0, NULL, 0},
+       { CHANNELMSG_OPENCHANNEL_RESULT,        1, vmbus_onopen_result,
+               sizeof(struct vmbus_channel_open_result)},
+       { CHANNELMSG_CLOSECHANNEL,              0, NULL, 0},
+       { CHANNELMSG_GPADL_HEADER,              0, NULL, 0},
+       { CHANNELMSG_GPADL_BODY,                0, NULL, 0},
+       { CHANNELMSG_GPADL_CREATED,             1, vmbus_ongpadl_created,
+               sizeof(struct vmbus_channel_gpadl_created)},
+       { CHANNELMSG_GPADL_TEARDOWN,            0, NULL, 0},
+       { CHANNELMSG_GPADL_TORNDOWN,            1, vmbus_ongpadl_torndown,
+               sizeof(struct vmbus_channel_gpadl_torndown) },
+       { CHANNELMSG_RELID_RELEASED,            0, NULL, 0},
+       { CHANNELMSG_INITIATE_CONTACT,          0, NULL, 0},
+       { CHANNELMSG_VERSION_RESPONSE,          1, vmbus_onversion_response,
+               sizeof(struct vmbus_channel_version_response)},
+       { CHANNELMSG_UNLOAD,                    0, NULL, 0},
+       { CHANNELMSG_UNLOAD_RESPONSE,           1, vmbus_unload_response, 0},
+       { CHANNELMSG_18,                        0, NULL, 0},
+       { CHANNELMSG_19,                        0, NULL, 0},
+       { CHANNELMSG_20,                        0, NULL, 0},
+       { CHANNELMSG_TL_CONNECT_REQUEST,        0, NULL, 0},
+       { CHANNELMSG_22,                        0, NULL, 0},
+       { CHANNELMSG_TL_CONNECT_RESULT,         0, NULL, 0},
 };
 
 /*

diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 70b30e223a578be42378342891541b560bae627b..ab560ac9c0407fdb9b4132581c0f1bb34ad81f61 100644 (file)
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -317,6 +317,7 @@ struct vmbus_channel_message_table_entry {
        enum vmbus_channel_message_type message_type;
        enum vmbus_message_handler_type handler_type;
        void (*message_handler)(struct vmbus_channel_message_header *msg);
+       u32 min_payload_len;
 };
 
 extern const struct vmbus_channel_message_table_entry

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 6e54adb1dd331b5b0d751c2d94a45aff39ccc895..3a05e1bc359c2ecd0f64e3489cccb418ee5a13f4 100644 (file)
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1082,6 +1082,12 @@ void vmbus_on_msg_dpc(unsigned long data)
        if (!entry->message_handler)
                goto msg_handled;
 
+       if (msg->header.payload_size < entry->min_payload_len) {
+               WARN_ONCE(1, "message too short: msgtype=%d len=%d\n",
+                         hdr->msgtype, msg->header.payload_size);
+               goto msg_handled;
+       }
+
        if (entry->handler_type == VMHT_BLOCKING) {
                ctx = kmalloc(sizeof(*ctx) + msg->header.payload_size,
                              GFP_ATOMIC);