Commit ac0f7d42 authored by Vitaly Kuznetsov's avatar Vitaly Kuznetsov Committed by Wei Liu

Drivers: hv: copy from message page only what's needed

Hyper-V Interrupt Message Page (SIMP) has 16 256-byte slots for
messages. Each message comes with a header (16 bytes) which specifies the
payload length (up to 240 bytes). vmbus_on_msg_dpc(), however, doesn't
look at the real message length and copies the whole slot to a temporary
buffer before passing it to message handlers. This is potentially dangerous
as hypervisor doesn't have to clean the whole slot when putting a new
message there and a message handler can get access to some data which
belongs to a previous message.

Note, this is not currently a problem because all message handlers are
in-kernel but eventually we may e.g. get this exported to userspace.

Note also, that this is not a performance critical path: messages (unlike
events) represent rare events so it doesn't really matter (from performance
point of view) if we copy too much.

Fix the issue by taking into account the real message length. The temporary
buffer allocated by vmbus_on_msg_dpc() remains fixed size for now. Also,
check that the supplied payload length is valid (<= 240 bytes).
Signed-off-by: default avatarVitaly Kuznetsov <vkuznets@redhat.com>
Reviewed-by: default avatarMichael Kelley <mikelley@microsoft.com>
Link: https://lore.kernel.org/r/20200406104154.45010-2-vkuznets@redhat.comSigned-off-by: default avatarWei Liu <wei.liu@kernel.org>
parent ae83d0b4
...@@ -1060,6 +1060,12 @@ void vmbus_on_msg_dpc(unsigned long data) ...@@ -1060,6 +1060,12 @@ void vmbus_on_msg_dpc(unsigned long data)
goto msg_handled; goto msg_handled;
} }
if (msg->header.payload_size > HV_MESSAGE_PAYLOAD_BYTE_COUNT) {
WARN_ONCE(1, "payload size is too large (%d)\n",
msg->header.payload_size);
goto msg_handled;
}
entry = &channel_message_table[hdr->msgtype]; entry = &channel_message_table[hdr->msgtype];
if (!entry->message_handler) if (!entry->message_handler)
...@@ -1071,7 +1077,8 @@ void vmbus_on_msg_dpc(unsigned long data) ...@@ -1071,7 +1077,8 @@ void vmbus_on_msg_dpc(unsigned long data)
return; return;
INIT_WORK(&ctx->work, vmbus_onmessage_work); INIT_WORK(&ctx->work, vmbus_onmessage_work);
memcpy(&ctx->msg, msg, sizeof(*msg)); memcpy(&ctx->msg, msg, sizeof(msg->header) +
msg->header.payload_size);
/* /*
* The host can generate a rescind message while we * The host can generate a rescind message while we
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment