Re: [PATCH v4] hv_utils: Add validation for untrusted Hyper-V values

[Andrea Parri]

On Mon, Nov 09, 2020 at 11:07:04AM +0100, Andrea Parri (Microsoft) wrote:
> From: Andres Beltran <lkmlabelt@xxxxxxxxx>
> 
> 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@xxxxxxxxx>
> Co-developed-by: Andrea Parri (Microsoft) <parri.andrea@xxxxxxxxx>
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@xxxxxxxxx>

Ping, other suggestions about this patch?

Thanks,
  Andrea


> ---
> Changes in v3:
> 	- Add size check for icframe_vercnt and icmsg_vercnt
> 
> Changes in v2:
> 	- Use ratelimited form of kernel logging to print error messages
> 
>  drivers/hv/channel_mgmt.c |  24 ++++-
>  drivers/hv/hv_fcopy.c     |  36 +++++--
>  drivers/hv/hv_kvp.c       | 122 ++++++++++++---------
>  drivers/hv/hv_snapshot.c  |  89 ++++++++-------
>  drivers/hv/hv_util.c      | 222 +++++++++++++++++++++++---------------
>  include/linux/hyperv.h    |   9 +-
>  6 files changed, 314 insertions(+), 188 deletions(-)
> 
> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index 1d44bb635bb84..5bc5eef5da159 100644
> --- a/drivers/hv/channel_mgmt.c
> +++ b/drivers/hv/channel_mgmt.c
> @@ -190,6 +190,7 @@ static u16 hv_get_dev_type(const struct vmbus_channel *channel)
>   * vmbus_prep_negotiate_resp() - Create default response for Negotiate message
>   * @icmsghdrp: Pointer to msg header structure
>   * @buf: Raw buffer channel data
> + * @buflen: Length of the raw buffer channel data.
>   * @fw_version: The framework versions we can support.
>   * @fw_vercnt: The size of @fw_version.
>   * @srv_version: The service versions we can support.
> @@ -202,8 +203,8 @@ static u16 hv_get_dev_type(const struct vmbus_channel *channel)
>   * Set up and fill in default negotiate response message.
>   * Mainly used by Hyper-V drivers.
>   */
> -bool vmbus_prep_negotiate_resp(struct icmsg_hdr *icmsghdrp,
> -				u8 *buf, const int *fw_version, int fw_vercnt,
> +bool vmbus_prep_negotiate_resp(struct icmsg_hdr *icmsghdrp, u8 *buf,
> +				u32 buflen, const int *fw_version, int fw_vercnt,
>  				const int *srv_version, int srv_vercnt,
>  				int *nego_fw_version, int *nego_srv_version)
>  {
> @@ -215,10 +216,14 @@ bool vmbus_prep_negotiate_resp(struct icmsg_hdr *icmsghdrp,
>  	bool found_match = false;
>  	struct icmsg_negotiate *negop;
>  
> +	/* Check that there's enough space for icframe_vercnt, icmsg_vercnt */
> +	if (buflen < ICMSG_HDR + offsetof(struct icmsg_negotiate, reserved)) {
> +		pr_err_ratelimited("Invalid icmsg negotiate\n");
> +		return false;
> +	}
> +
>  	icmsghdrp->icmsgsize = 0x10;
> -	negop = (struct icmsg_negotiate *)&buf[
> -		sizeof(struct vmbuspipe_hdr) +
> -		sizeof(struct icmsg_hdr)];
> +	negop = (struct icmsg_negotiate *)&buf[ICMSG_HDR];
>  
>  	icframe_major = negop->icframe_vercnt;
>  	icframe_minor = 0;
> @@ -226,6 +231,15 @@ bool vmbus_prep_negotiate_resp(struct icmsg_hdr *icmsghdrp,
>  	icmsg_major = negop->icmsg_vercnt;
>  	icmsg_minor = 0;
>  
> +	/* Validate negop packet */
> +	if (icframe_major > IC_VERSION_NEGOTIATION_MAX_VER_COUNT ||
> +	    icmsg_major > IC_VERSION_NEGOTIATION_MAX_VER_COUNT ||
> +	    ICMSG_NEGOTIATE_PKT_SIZE(icframe_major, icmsg_major) > buflen) {
> +		pr_err_ratelimited("Invalid icmsg negotiate - icframe_major: %u, icmsg_major: %u\n",
> +				   icframe_major, icmsg_major);
> +		goto fw_error;
> +	}
> +
>  	/*
>  	 * Select the framework version number we will
>  	 * support.
> diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c
> index 5040d7e0cd9e9..59ce85e00a028 100644
> --- a/drivers/hv/hv_fcopy.c
> +++ b/drivers/hv/hv_fcopy.c
> @@ -235,15 +235,27 @@ void hv_fcopy_onchannelcallback(void *context)
>  	if (fcopy_transaction.state > HVUTIL_READY)
>  		return;
>  
> -	vmbus_recvpacket(channel, recv_buffer, HV_HYP_PAGE_SIZE * 2, &recvlen,
> -			 &requestid);
> -	if (recvlen <= 0)
> +	if (vmbus_recvpacket(channel, recv_buffer, HV_HYP_PAGE_SIZE * 2, &recvlen, &requestid)) {
> +		pr_err_ratelimited("Fcopy request received. Could not read into recv buf\n");
>  		return;
> +	}
> +
> +	if (!recvlen)
> +		return;
> +
> +	/* Ensure recvlen is big enough to read header data */
> +	if (recvlen < ICMSG_HDR) {
> +		pr_err_ratelimited("Fcopy request received. Packet length too small: %d\n",
> +				   recvlen);
> +		return;
> +	}
>  
>  	icmsghdr = (struct icmsg_hdr *)&recv_buffer[
>  			sizeof(struct vmbuspipe_hdr)];
> +
>  	if (icmsghdr->icmsgtype == ICMSGTYPE_NEGOTIATE) {
> -		if (vmbus_prep_negotiate_resp(icmsghdr, recv_buffer,
> +		if (vmbus_prep_negotiate_resp(icmsghdr,
> +				recv_buffer, recvlen,
>  				fw_versions, FW_VER_COUNT,
>  				fcopy_versions, FCOPY_VER_COUNT,
>  				NULL, &fcopy_srv_version)) {
> @@ -252,10 +264,14 @@ void hv_fcopy_onchannelcallback(void *context)
>  				fcopy_srv_version >> 16,
>  				fcopy_srv_version & 0xFFFF);
>  		}
> -	} else {
> -		fcopy_msg = (struct hv_fcopy_hdr *)&recv_buffer[
> -				sizeof(struct vmbuspipe_hdr) +
> -				sizeof(struct icmsg_hdr)];
> +	} else if (icmsghdr->icmsgtype == ICMSGTYPE_FCOPY) {
> +		/* Ensure recvlen is big enough to contain hv_fcopy_hdr */
> +		if (recvlen < ICMSG_HDR + sizeof(struct hv_fcopy_hdr)) {
> +			pr_err_ratelimited("Invalid Fcopy hdr. Packet length too small: %u\n",
> +					   recvlen);
> +			return;
> +		}
> +		fcopy_msg = (struct hv_fcopy_hdr *)&recv_buffer[ICMSG_HDR];
>  
>  		/*
>  		 * Stash away this global state for completing the
> @@ -280,6 +296,10 @@ void hv_fcopy_onchannelcallback(void *context)
>  		schedule_delayed_work(&fcopy_timeout_work,
>  				      HV_UTIL_TIMEOUT * HZ);
>  		return;
> +	} else {
> +		pr_err_ratelimited("Fcopy request received. Invalid msg type: %d\n",
> +				   icmsghdr->icmsgtype);
> +		return;
>  	}
>  	icmsghdr->icflags = ICMSGHDRFLAG_TRANSACTION | ICMSGHDRFLAG_RESPONSE;
>  	vmbus_sendpacket(channel, recv_buffer, recvlen, requestid,
> diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
> index 754d35a25a1cc..b49962d312cef 100644
> --- a/drivers/hv/hv_kvp.c
> +++ b/drivers/hv/hv_kvp.c
> @@ -662,71 +662,87 @@ void hv_kvp_onchannelcallback(void *context)
>  	if (kvp_transaction.state > HVUTIL_READY)
>  		return;
>  
> -	vmbus_recvpacket(channel, recv_buffer, HV_HYP_PAGE_SIZE * 4, &recvlen,
> -			 &requestid);
> -
> -	if (recvlen > 0) {
> -		icmsghdrp = (struct icmsg_hdr *)&recv_buffer[
> -			sizeof(struct vmbuspipe_hdr)];
> -
> -		if (icmsghdrp->icmsgtype == ICMSGTYPE_NEGOTIATE) {
> -			if (vmbus_prep_negotiate_resp(icmsghdrp,
> -				 recv_buffer, fw_versions, FW_VER_COUNT,
> -				 kvp_versions, KVP_VER_COUNT,
> -				 NULL, &kvp_srv_version)) {
> -				pr_info("KVP IC version %d.%d\n",
> -					kvp_srv_version >> 16,
> -					kvp_srv_version & 0xFFFF);
> -			}
> -		} else {
> -			kvp_msg = (struct hv_kvp_msg *)&recv_buffer[
> -				sizeof(struct vmbuspipe_hdr) +
> -				sizeof(struct icmsg_hdr)];
> +	if (vmbus_recvpacket(channel, recv_buffer, HV_HYP_PAGE_SIZE * 4, &recvlen, &requestid)) {
> +		pr_err_ratelimited("KVP request received. Could not read into recv buf\n");
> +		return;
> +	}
>  
> -			/*
> -			 * Stash away this global state for completing the
> -			 * transaction; note transactions are serialized.
> -			 */
> +	if (!recvlen)
> +		return;
>  
> -			kvp_transaction.recv_len = recvlen;
> -			kvp_transaction.recv_req_id = requestid;
> -			kvp_transaction.kvp_msg = kvp_msg;
> +	/* Ensure recvlen is big enough to read header data */
> +	if (recvlen < ICMSG_HDR) {
> +		pr_err_ratelimited("KVP request received. Packet length too small: %d\n",
> +				   recvlen);
> +		return;
> +	}
>  
> -			if (kvp_transaction.state < HVUTIL_READY) {
> -				/* Userspace is not registered yet */
> -				kvp_respond_to_host(NULL, HV_E_FAIL);
> -				return;
> -			}
> -			kvp_transaction.state = HVUTIL_HOSTMSG_RECEIVED;
> +	icmsghdrp = (struct icmsg_hdr *)&recv_buffer[sizeof(struct vmbuspipe_hdr)];
> +
> +	if (icmsghdrp->icmsgtype == ICMSGTYPE_NEGOTIATE) {
> +		if (vmbus_prep_negotiate_resp(icmsghdrp,
> +				recv_buffer, recvlen,
> +				fw_versions, FW_VER_COUNT,
> +				kvp_versions, KVP_VER_COUNT,
> +				NULL, &kvp_srv_version)) {
> +			pr_info("KVP IC version %d.%d\n",
> +				kvp_srv_version >> 16,
> +				kvp_srv_version & 0xFFFF);
> +		}
> +	} else if (icmsghdrp->icmsgtype == ICMSGTYPE_KVPEXCHANGE) {
> +		/*
> +		 * recvlen is not checked against sizeof(struct kvp_msg) because kvp_msg contains
> +		 * a union of structs and the msg type received is not known. Code using this
> +		 * struct should provide validation when accessing its fields.
> +		 */
> +		kvp_msg = (struct hv_kvp_msg *)&recv_buffer[ICMSG_HDR];
>  
> -			/*
> -			 * Get the information from the
> -			 * user-mode component.
> -			 * component. This transaction will be
> -			 * completed when we get the value from
> -			 * the user-mode component.
> -			 * Set a timeout to deal with
> -			 * user-mode not responding.
> -			 */
> -			schedule_work(&kvp_sendkey_work);
> -			schedule_delayed_work(&kvp_timeout_work,
> -					      HV_UTIL_TIMEOUT * HZ);
> +		/*
> +		 * Stash away this global state for completing the
> +		 * transaction; note transactions are serialized.
> +		 */
>  
> -			return;
> +		kvp_transaction.recv_len = recvlen;
> +		kvp_transaction.recv_req_id = requestid;
> +		kvp_transaction.kvp_msg = kvp_msg;
>  
> +		if (kvp_transaction.state < HVUTIL_READY) {
> +			/* Userspace is not registered yet */
> +			kvp_respond_to_host(NULL, HV_E_FAIL);
> +			return;
>  		}
> +		kvp_transaction.state = HVUTIL_HOSTMSG_RECEIVED;
>  
> -		icmsghdrp->icflags = ICMSGHDRFLAG_TRANSACTION
> -			| ICMSGHDRFLAG_RESPONSE;
> +		/*
> +		 * Get the information from the
> +		 * user-mode component.
> +		 * component. This transaction will be
> +		 * completed when we get the value from
> +		 * the user-mode component.
> +		 * Set a timeout to deal with
> +		 * user-mode not responding.
> +		 */
> +		schedule_work(&kvp_sendkey_work);
> +		schedule_delayed_work(&kvp_timeout_work,
> +					HV_UTIL_TIMEOUT * HZ);
>  
> -		vmbus_sendpacket(channel, recv_buffer,
> -				       recvlen, requestid,
> -				       VM_PKT_DATA_INBAND, 0);
> +		return;
>  
> -		host_negotiatied = NEGO_FINISHED;
> -		hv_poll_channel(kvp_transaction.recv_channel, kvp_poll_wrapper);
> +	} else {
> +		pr_err_ratelimited("KVP request received. Invalid msg type: %d\n",
> +				   icmsghdrp->icmsgtype);
> +		return;
>  	}
>  
> +	icmsghdrp->icflags = ICMSGHDRFLAG_TRANSACTION
> +		| ICMSGHDRFLAG_RESPONSE;
> +
> +	vmbus_sendpacket(channel, recv_buffer,
> +			 recvlen, requestid,
> +			 VM_PKT_DATA_INBAND, 0);
> +
> +	host_negotiatied = NEGO_FINISHED;
> +	hv_poll_channel(kvp_transaction.recv_channel, kvp_poll_wrapper);
>  }
>  
>  static void kvp_on_reset(void)
> diff --git a/drivers/hv/hv_snapshot.c b/drivers/hv/hv_snapshot.c
> index 783779e4cc1a5..2267bd4c34725 100644
> --- a/drivers/hv/hv_snapshot.c
> +++ b/drivers/hv/hv_snapshot.c
> @@ -298,49 +298,64 @@ void hv_vss_onchannelcallback(void *context)
>  	if (vss_transaction.state > HVUTIL_READY)
>  		return;
>  
> -	vmbus_recvpacket(channel, recv_buffer, HV_HYP_PAGE_SIZE * 2, &recvlen,
> -			 &requestid);
> -
> -	if (recvlen > 0) {
> -		icmsghdrp = (struct icmsg_hdr *)&recv_buffer[
> -			sizeof(struct vmbuspipe_hdr)];
> -
> -		if (icmsghdrp->icmsgtype == ICMSGTYPE_NEGOTIATE) {
> -			if (vmbus_prep_negotiate_resp(icmsghdrp,
> -				 recv_buffer, fw_versions, FW_VER_COUNT,
> -				 vss_versions, VSS_VER_COUNT,
> -				 NULL, &vss_srv_version)) {
> -
> -				pr_info("VSS IC version %d.%d\n",
> -					vss_srv_version >> 16,
> -					vss_srv_version & 0xFFFF);
> -			}
> -		} else {
> -			vss_msg = (struct hv_vss_msg *)&recv_buffer[
> -				sizeof(struct vmbuspipe_hdr) +
> -				sizeof(struct icmsg_hdr)];
> -
> -			/*
> -			 * Stash away this global state for completing the
> -			 * transaction; note transactions are serialized.
> -			 */
> -
> -			vss_transaction.recv_len = recvlen;
> -			vss_transaction.recv_req_id = requestid;
> -			vss_transaction.msg = (struct hv_vss_msg *)vss_msg;
> -
> -			schedule_work(&vss_handle_request_work);
> +	if (vmbus_recvpacket(channel, recv_buffer, HV_HYP_PAGE_SIZE * 2, &recvlen, &requestid)) {
> +		pr_err_ratelimited("VSS request received. Could not read into recv buf\n");
> +		return;
> +	}
> +
> +	if (!recvlen)
> +		return;
> +
> +	/* Ensure recvlen is big enough to read header data */
> +	if (recvlen < ICMSG_HDR) {
> +		pr_err_ratelimited("VSS request received. Packet length too small: %d\n",
> +				   recvlen);
> +		return;
> +	}
> +
> +	icmsghdrp = (struct icmsg_hdr *)&recv_buffer[sizeof(struct vmbuspipe_hdr)];
> +
> +	if (icmsghdrp->icmsgtype == ICMSGTYPE_NEGOTIATE) {
> +		if (vmbus_prep_negotiate_resp(icmsghdrp,
> +				recv_buffer, recvlen,
> +				fw_versions, FW_VER_COUNT,
> +				vss_versions, VSS_VER_COUNT,
> +				NULL, &vss_srv_version)) {
> +
> +			pr_info("VSS IC version %d.%d\n",
> +				vss_srv_version >> 16,
> +				vss_srv_version & 0xFFFF);
> +		}
> +	} else if (icmsghdrp->icmsgtype == ICMSGTYPE_VSS) {
> +		/* Ensure recvlen is big enough to contain hv_vss_msg */
> +		if (recvlen < ICMSG_HDR + sizeof(struct hv_vss_msg)) {
> +			pr_err_ratelimited("Invalid VSS msg. Packet length too small: %u\n",
> +					   recvlen);
>  			return;
>  		}
> +		vss_msg = (struct hv_vss_msg *)&recv_buffer[ICMSG_HDR];
> +
> +		/*
> +		 * Stash away this global state for completing the
> +		 * transaction; note transactions are serialized.
> +		 */
>  
> -		icmsghdrp->icflags = ICMSGHDRFLAG_TRANSACTION
> -			| ICMSGHDRFLAG_RESPONSE;
> +		vss_transaction.recv_len = recvlen;
> +		vss_transaction.recv_req_id = requestid;
> +		vss_transaction.msg = (struct hv_vss_msg *)vss_msg;
>  
> -		vmbus_sendpacket(channel, recv_buffer,
> -				       recvlen, requestid,
> -				       VM_PKT_DATA_INBAND, 0);
> +		schedule_work(&vss_handle_request_work);
> +		return;
> +	} else {
> +		pr_err_ratelimited("VSS request received. Invalid msg type: %d\n",
> +				   icmsghdrp->icmsgtype);
> +		return;
>  	}
>  
> +	icmsghdrp->icflags = ICMSGHDRFLAG_TRANSACTION |
> +		ICMSGHDRFLAG_RESPONSE;
> +	vmbus_sendpacket(channel, recv_buffer, recvlen, requestid,
> +			 VM_PKT_DATA_INBAND, 0);
>  }
>  
>  static void vss_on_reset(void)
> diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
> index 05566ecdbe4b4..34f3e789cc9a2 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;
> -
> -		vmbus_sendpacket(channel, shut_txf_buf,
> -				       recvlen, requestid,
> -				       VM_PKT_DATA_INBAND, 0);
> +		shutdown_msg = (struct shutdown_msg_data *)&shut_txf_buf[ICMSG_HDR];
> +
> +		/*
> +		 * 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);
>  	}
>  }
>  
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index 5ddb479c4d4cb..696857aa038c0 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -1471,6 +1471,7 @@ void vmbus_free_mmio(resource_size_t start, resource_size_t size);
>  #define ICMSGTYPE_SHUTDOWN		3
>  #define ICMSGTYPE_TIMESYNC		4
>  #define ICMSGTYPE_VSS			5
> +#define ICMSGTYPE_FCOPY			7
>  
>  #define ICMSGHDRFLAG_TRANSACTION	1
>  #define ICMSGHDRFLAG_REQUEST		2
> @@ -1514,6 +1515,12 @@ struct icmsg_hdr {
>  	u8 reserved[2];
>  } __packed;
>  
> +#define IC_VERSION_NEGOTIATION_MAX_VER_COUNT 100
> +#define ICMSG_HDR (sizeof(struct vmbuspipe_hdr) + sizeof(struct icmsg_hdr))
> +#define ICMSG_NEGOTIATE_PKT_SIZE(icframe_vercnt, icmsg_vercnt) \
> +	(ICMSG_HDR + offsetof(struct icmsg_negotiate, icversion_data) + \
> +	 (((icframe_vercnt) + (icmsg_vercnt)) * sizeof(struct ic_version)))
> +
>  struct icmsg_negotiate {
>  	u16 icframe_vercnt;
>  	u16 icmsg_vercnt;
> @@ -1569,7 +1576,7 @@ struct hyperv_service_callback {
>  };
>  
>  #define MAX_SRV_VER	0x7ffffff
> -extern bool vmbus_prep_negotiate_resp(struct icmsg_hdr *icmsghdrp, u8 *buf,
> +extern bool vmbus_prep_negotiate_resp(struct icmsg_hdr *icmsghdrp, u8 *buf, u32 buflen,
>  				const int *fw_version, int fw_vercnt,
>  				const int *srv_version, int srv_vercnt,
>  				int *nego_fw_version, int *nego_srv_version);
> -- 
> 2.25.1
>