linux-kernel - RE: [PATCH] hv_utils: drain the timesync packets on onchannelcallback

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <MW2PR2101MB1052254C5ED7C587E548DB3AD75A0@MW2PR2101MB1052.namprd21.prod.outlook.com>
Date:   Thu, 20 Aug 2020 18:31:12 +0000
From:   Michael Kelley <mikelley@...rosoft.com>
To:     Vineeth Pillai <viremana@...ux.microsoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        Wei Liu <wei.liu@...nel.org>
CC:     KY Srinivasan <kys@...rosoft.com>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] hv_utils: drain the timesync packets on onchannelcallback

From: Vineeth Pillai <viremana@...ux.microsoft.com> Sent: Wednesday, August 19, 2020 10:48 AM
> 
> There could be instances where a system stall prevents the timesync
> packaets to be consumed. And this might lead to more than one packet
> pending in the ring buffer. Current code empties one packet per callback
> and it might be a stale one. So drain all the packets from ring buffer
> on each callback.
> 
> Signed-off-by: Vineeth Pillai <viremana@...ux.microsoft.com>
> ---
>  drivers/hv/hv_util.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
> index 1357861fd8ae..c0491b727fd5 100644
> --- a/drivers/hv/hv_util.c
> +++ b/drivers/hv/hv_util.c
> @@ -387,10 +387,23 @@ static void timesync_onchannelcallback(void *context)
>  	struct ictimesync_ref_data *refdata;
>  	u8 *time_txf_buf = util_timesynch.recv_buffer;
> 
> -	vmbus_recvpacket(channel, time_txf_buf,
> -			 HV_HYP_PAGE_SIZE, &recvlen, &requestid);
> +	/*
> +	 * Drain the ring buffer and use the last packet to update
> +	 * host_ts
> +	 */
> +	while (1) {
> +		int ret = vmbus_recvpacket(channel, time_txf_buf,
> +					   HV_HYP_PAGE_SIZE, &recvlen,
> +					   &requestid);
> +		if (ret) {
> +			pr_warn("TimeSync IC pkt recv failed (Err: %d)\n",
> +				ret);

Let's use pr_warn_once().

If there's a packet at the head of the ring buffer that specifies a bogus length,
we could take the error path.  But the bad packet stays at the head of the ring buffer,
so if we end up back here again, we'll spit out the same error message.  We
actually should not end up here again because Hyper-V shouldn't interrupt
when adding a packet to a non-empty ring buffer, but who knows what might
happen.

> +			break;
> +		}
> +
> +		if (!recvlen)
> +			break;
> 
> -	if (recvlen > 0) {
>  		icmsghdrp = (struct icmsg_hdr *)&time_txf_buf[
>  				sizeof(struct vmbuspipe_hdr)];
> 
> --
> 2.17.1