hv_utils: Add validation for untrusted Hyper-V values
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 indexing off the end of the icversion_data
array in vmbus_prep_negotiate_resp().
Signed-off-by: Andres Beltran <lkmlabelt@gmail.com>
Co-developed-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
Link: https://lore.kernel.org/r/20201109100704.9152-1-parri.andrea@gmail.com
Signed-off-by: Wei Liu <wei.liu@kernel.org>
diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index 05566ec..34f3e78 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -195,73 +195,88 @@ static void shutdown_onchannelcallback(void *context)
struct icmsg_hdr *icmsghdrp;
- vmbus_recvpacket(channel, shut_txf_buf,
- HV_HYP_PAGE_SIZE, &recvlen, &requestid);
+ if (vmbus_recvpacket(channel, shut_txf_buf, HV_HYP_PAGE_SIZE, &recvlen, &requestid)) {
+ pr_err_ratelimited("Shutdown request received. Could not read into shut txf buf\n");
+ return;
+ }
- if (recvlen > 0) {
- icmsghdrp = (struct icmsg_hdr *)&shut_txf_buf[
- sizeof(struct vmbuspipe_hdr)];
+ if (!recvlen)
+ return;
- if (icmsghdrp->icmsgtype == ICMSGTYPE_NEGOTIATE) {
- if (vmbus_prep_negotiate_resp(icmsghdrp, shut_txf_buf,
- fw_versions, FW_VER_COUNT,
- sd_versions, SD_VER_COUNT,
- NULL, &sd_srv_version)) {
- pr_info("Shutdown IC version %d.%d\n",
- sd_srv_version >> 16,
- sd_srv_version & 0xFFFF);
- }
- } else {
- shutdown_msg =
- (struct shutdown_msg_data *)&shut_txf_buf[
- sizeof(struct vmbuspipe_hdr) +
- sizeof(struct icmsg_hdr)];
+ /* Ensure recvlen is big enough to read header data */
+ if (recvlen < ICMSG_HDR) {
+ pr_err_ratelimited("Shutdown request received. Packet length too small: %d\n",
+ recvlen);
+ return;
+ }
- /*
- * shutdown_msg->flags can be 0(shut down), 2(reboot),
- * or 4(hibernate). It may bitwise-OR 1, which means
- * performing the request by force. Linux always tries
- * to perform the request by force.
- */
- switch (shutdown_msg->flags) {
- case 0:
- case 1:
- icmsghdrp->status = HV_S_OK;
- work = &shutdown_work;
- pr_info("Shutdown request received -"
- " graceful shutdown initiated\n");
- break;
- case 2:
- case 3:
- icmsghdrp->status = HV_S_OK;
- work = &restart_work;
- pr_info("Restart request received -"
- " graceful restart initiated\n");
- break;
- case 4:
- case 5:
- pr_info("Hibernation request received\n");
- icmsghdrp->status = hibernation_supported ?
- HV_S_OK : HV_E_FAIL;
- if (hibernation_supported)
- work = &hibernate_context.work;
- break;
- default:
- icmsghdrp->status = HV_E_FAIL;
- pr_info("Shutdown request received -"
- " Invalid request\n");
- break;
- }
+ icmsghdrp = (struct icmsg_hdr *)&shut_txf_buf[sizeof(struct vmbuspipe_hdr)];
+
+ if (icmsghdrp->icmsgtype == ICMSGTYPE_NEGOTIATE) {
+ if (vmbus_prep_negotiate_resp(icmsghdrp,
+ shut_txf_buf, recvlen,
+ fw_versions, FW_VER_COUNT,
+ sd_versions, SD_VER_COUNT,
+ NULL, &sd_srv_version)) {
+ pr_info("Shutdown IC version %d.%d\n",
+ sd_srv_version >> 16,
+ sd_srv_version & 0xFFFF);
+ }
+ } else if (icmsghdrp->icmsgtype == ICMSGTYPE_SHUTDOWN) {
+ /* Ensure recvlen is big enough to contain shutdown_msg_data struct */
+ if (recvlen < ICMSG_HDR + sizeof(struct shutdown_msg_data)) {
+ pr_err_ratelimited("Invalid shutdown msg data. Packet length too small: %u\n",
+ recvlen);
+ return;
}
- icmsghdrp->icflags = ICMSGHDRFLAG_TRANSACTION
- | ICMSGHDRFLAG_RESPONSE;
+ shutdown_msg = (struct shutdown_msg_data *)&shut_txf_buf[ICMSG_HDR];
- vmbus_sendpacket(channel, shut_txf_buf,
- recvlen, requestid,
- VM_PKT_DATA_INBAND, 0);
+ /*
+ * shutdown_msg->flags can be 0(shut down), 2(reboot),
+ * or 4(hibernate). It may bitwise-OR 1, which means
+ * performing the request by force. Linux always tries
+ * to perform the request by force.
+ */
+ switch (shutdown_msg->flags) {
+ case 0:
+ case 1:
+ icmsghdrp->status = HV_S_OK;
+ work = &shutdown_work;
+ pr_info("Shutdown request received - graceful shutdown initiated\n");
+ break;
+ case 2:
+ case 3:
+ icmsghdrp->status = HV_S_OK;
+ work = &restart_work;
+ pr_info("Restart request received - graceful restart initiated\n");
+ break;
+ case 4:
+ case 5:
+ pr_info("Hibernation request received\n");
+ icmsghdrp->status = hibernation_supported ?
+ HV_S_OK : HV_E_FAIL;
+ if (hibernation_supported)
+ work = &hibernate_context.work;
+ break;
+ default:
+ icmsghdrp->status = HV_E_FAIL;
+ pr_info("Shutdown request received - Invalid request\n");
+ break;
+ }
+ } else {
+ icmsghdrp->status = HV_E_FAIL;
+ pr_err_ratelimited("Shutdown request received. Invalid msg type: %d\n",
+ icmsghdrp->icmsgtype);
}
+ icmsghdrp->icflags = ICMSGHDRFLAG_TRANSACTION
+ | ICMSGHDRFLAG_RESPONSE;
+
+ vmbus_sendpacket(channel, shut_txf_buf,
+ recvlen, requestid,
+ VM_PKT_DATA_INBAND, 0);
+
if (work)
schedule_work(work);
}
@@ -396,19 +411,27 @@ static void timesync_onchannelcallback(void *context)
HV_HYP_PAGE_SIZE, &recvlen,
&requestid);
if (ret) {
- pr_warn_once("TimeSync IC pkt recv failed (Err: %d)\n",
- ret);
+ pr_err_ratelimited("TimeSync IC pkt recv failed (Err: %d)\n",
+ ret);
break;
}
if (!recvlen)
break;
+ /* Ensure recvlen is big enough to read header data */
+ if (recvlen < ICMSG_HDR) {
+ pr_err_ratelimited("Timesync request received. Packet length too small: %d\n",
+ recvlen);
+ break;
+ }
+
icmsghdrp = (struct icmsg_hdr *)&time_txf_buf[
sizeof(struct vmbuspipe_hdr)];
if (icmsghdrp->icmsgtype == ICMSGTYPE_NEGOTIATE) {
- if (vmbus_prep_negotiate_resp(icmsghdrp, time_txf_buf,
+ if (vmbus_prep_negotiate_resp(icmsghdrp,
+ time_txf_buf, recvlen,
fw_versions, FW_VER_COUNT,
ts_versions, TS_VER_COUNT,
NULL, &ts_srv_version)) {
@@ -416,33 +439,44 @@ static void timesync_onchannelcallback(void *context)
ts_srv_version >> 16,
ts_srv_version & 0xFFFF);
}
- } else {
+ } else if (icmsghdrp->icmsgtype == ICMSGTYPE_TIMESYNC) {
if (ts_srv_version > TS_VERSION_3) {
- refdata = (struct ictimesync_ref_data *)
- &time_txf_buf[
- sizeof(struct vmbuspipe_hdr) +
- sizeof(struct icmsg_hdr)];
+ /* Ensure recvlen is big enough to read ictimesync_ref_data */
+ if (recvlen < ICMSG_HDR + sizeof(struct ictimesync_ref_data)) {
+ pr_err_ratelimited("Invalid ictimesync ref data. Length too small: %u\n",
+ recvlen);
+ break;
+ }
+ refdata = (struct ictimesync_ref_data *)&time_txf_buf[ICMSG_HDR];
adj_guesttime(refdata->parenttime,
refdata->vmreferencetime,
refdata->flags);
} else {
- timedatap = (struct ictimesync_data *)
- &time_txf_buf[
- sizeof(struct vmbuspipe_hdr) +
- sizeof(struct icmsg_hdr)];
+ /* Ensure recvlen is big enough to read ictimesync_data */
+ if (recvlen < ICMSG_HDR + sizeof(struct ictimesync_data)) {
+ pr_err_ratelimited("Invalid ictimesync data. Length too small: %u\n",
+ recvlen);
+ break;
+ }
+ timedatap = (struct ictimesync_data *)&time_txf_buf[ICMSG_HDR];
+
adj_guesttime(timedatap->parenttime,
hv_read_reference_counter(),
timedatap->flags);
}
+ } else {
+ icmsghdrp->status = HV_E_FAIL;
+ pr_err_ratelimited("Timesync request received. Invalid msg type: %d\n",
+ icmsghdrp->icmsgtype);
}
icmsghdrp->icflags = ICMSGHDRFLAG_TRANSACTION
| ICMSGHDRFLAG_RESPONSE;
vmbus_sendpacket(channel, time_txf_buf,
- recvlen, requestid,
- VM_PKT_DATA_INBAND, 0);
+ recvlen, requestid,
+ VM_PKT_DATA_INBAND, 0);
}
}
@@ -462,18 +496,28 @@ static void heartbeat_onchannelcallback(void *context)
while (1) {
- vmbus_recvpacket(channel, hbeat_txf_buf,
- HV_HYP_PAGE_SIZE, &recvlen, &requestid);
+ if (vmbus_recvpacket(channel, hbeat_txf_buf, HV_HYP_PAGE_SIZE,
+ &recvlen, &requestid)) {
+ pr_err_ratelimited("Heartbeat request received. Could not read into hbeat txf buf\n");
+ return;
+ }
if (!recvlen)
break;
+ /* Ensure recvlen is big enough to read header data */
+ if (recvlen < ICMSG_HDR) {
+ pr_err_ratelimited("Hearbeat request received. Packet length too small: %d\n",
+ recvlen);
+ break;
+ }
+
icmsghdrp = (struct icmsg_hdr *)&hbeat_txf_buf[
sizeof(struct vmbuspipe_hdr)];
if (icmsghdrp->icmsgtype == ICMSGTYPE_NEGOTIATE) {
if (vmbus_prep_negotiate_resp(icmsghdrp,
- hbeat_txf_buf,
+ hbeat_txf_buf, recvlen,
fw_versions, FW_VER_COUNT,
hb_versions, HB_VER_COUNT,
NULL, &hb_srv_version)) {
@@ -482,21 +526,31 @@ static void heartbeat_onchannelcallback(void *context)
hb_srv_version >> 16,
hb_srv_version & 0xFFFF);
}
- } else {
- heartbeat_msg =
- (struct heartbeat_msg_data *)&hbeat_txf_buf[
- sizeof(struct vmbuspipe_hdr) +
- sizeof(struct icmsg_hdr)];
+ } else if (icmsghdrp->icmsgtype == ICMSGTYPE_HEARTBEAT) {
+ /*
+ * Ensure recvlen is big enough to read seq_num. Reserved area is not
+ * included in the check as the host may not fill it up entirely
+ */
+ if (recvlen < ICMSG_HDR + sizeof(u64)) {
+ pr_err_ratelimited("Invalid heartbeat msg data. Length too small: %u\n",
+ recvlen);
+ break;
+ }
+ heartbeat_msg = (struct heartbeat_msg_data *)&hbeat_txf_buf[ICMSG_HDR];
heartbeat_msg->seq_num += 1;
+ } else {
+ icmsghdrp->status = HV_E_FAIL;
+ pr_err_ratelimited("Heartbeat request received. Invalid msg type: %d\n",
+ icmsghdrp->icmsgtype);
}
icmsghdrp->icflags = ICMSGHDRFLAG_TRANSACTION
| ICMSGHDRFLAG_RESPONSE;
vmbus_sendpacket(channel, hbeat_txf_buf,
- recvlen, requestid,
- VM_PKT_DATA_INBAND, 0);
+ recvlen, requestid,
+ VM_PKT_DATA_INBAND, 0);
}
}
