linux-kernel - Re: [PATCH v2 3/3] Drivers: hv: vmbus: Check for pending channel interrupts before taking a CPU offline

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] [day] [month] [year] [list]

Date:   Thu, 15 Apr 2021 13:39:18 +0200
From:   Andrea Parri <parri.andrea@...il.com>
To:     Michael Kelley <mikelley@...rosoft.com>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        KY Srinivasan <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        "wei.liu@...nel.org" <wei.liu@...nel.org>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>
Subject: Re: [PATCH v2 3/3] Drivers: hv: vmbus: Check for pending channel
 interrupts before taking a CPU offline

> > @@ -336,6 +372,19 @@ int hv_synic_cleanup(unsigned int cpu)
> >  	if (channel_found && vmbus_connection.conn_state == CONNECTED)
> >  		return -EBUSY;
> > 
> > +	if (vmbus_proto_version >= VERSION_WIN10_V4_1) {
> > +		/*
> > +		 * channel_found == false means that any channels that were previously
> > +		 * assigned to the CPU have been reassigned elsewhere with a call of
> > +		 * vmbus_send_modifychannel().  Scan the event flags page looking for
> > +		 * bits that are set and waiting with a timeout for vmbus_chan_sched()
> > +		 * to process such bits.  If bits are still set after this operation
> > +		 * and VMBus is connected, fail the CPU offlining operation.
> > +		 */
> > +		if (hv_synic_event_pending() && vmbus_connection.conn_state == CONNECTED)
> > +			return -EBUSY;
> > +	}
> > +
> 
> Perhaps the test for conn_state == CONNECTED could be factored out as follows.  If we're
> not CONNECTED (i.e., in the panic or kexec path) we should be able to also skip the search
> for channels that are bound to the CPU since we will ignore the result anyway.
> 
> 	if (vmbus_connection.conn_state != CONNECTED)
> 		goto always_cleanup;
> 
> 	if (cpu == VMBUS_CONNECT_CPU)
> 		return -EBUSY;
> 
> 	[Code to search for channels that are bound to the CPU we're about to clean up]
> 	
> 	if (channel_found)
> 		return -EBUSY;
> 
> 	/*
> 	 * channel_found == false means that any channels that were previously
> 	 * assigned to the CPU have been reassigned elsewhere with a call of
> 	 * vmbus_send_modifychannel().  Scan the event flags page looking for
> 	 * bits that are set and waiting with a timeout for vmbus_chan_sched()
> 	 * to process such bits.  If bits are still set after this operation
> 	 * and VMBus is connected, fail the CPU offlining operation.
> 	 */
> 	if (vmbus_proto_version >= VERSION_WIN10_V4_1 && hv_synic_event_pending())
> 		return -EBUSY;
> 
> always_cleanup:

Agreed, applied.  Thank you for the suggestion,

  Andrea