linux-kernel - Re: [PATCH 2/2] x86/hyperv: remove on-stack cpumask from hv_send_ipi_mask

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 for Android: free password hash cracker in your pocket

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

Date:   Fri, 10 Sep 2021 18:36:32 +0000
From:   Wei Liu <wei.liu@...nel.org>
To:     Michael Kelley <mikelley@...rosoft.com>
Cc:     Wei Liu <wei.liu@...nel.org>,
        Linux on Hyper-V List <linux-hyperv@...r.kernel.org>,
        KY Srinivasan <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Dexuan Cui <decui@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>,
        "open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)" 
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] x86/hyperv: remove on-stack cpumask from
 hv_send_ipi_mask_allbutself

On Fri, Sep 10, 2021 at 05:25:15PM +0000, Michael Kelley wrote:
> From: Wei Liu <wei.liu@...nel.org> Sent: Wednesday, September 8, 2021 12:43 PM
> > 
[...]
> > -static bool __send_ipi_mask(const struct cpumask *mask, int vector)
> > +static bool __send_ipi_mask(const struct cpumask *mask, int vector,
> > +		bool exclude_self)
> >  {
> > -	int cur_cpu, vcpu;
> > +	int cur_cpu, vcpu, this_cpu = smp_processor_id();
> >  	struct hv_send_ipi ipi_arg;
> >  	u64 status;
> > 
> > @@ -172,6 +177,8 @@ static bool __send_ipi_mask(const struct cpumask *mask, int vector)
> >  	ipi_arg.cpu_mask = 0;
> > 
> >  	for_each_cpu(cur_cpu, mask) {
> > +		if (exclude_self && cur_cpu == this_cpu)
> > +			continue;
> >  		vcpu = hv_cpu_number_to_vp_number(cur_cpu);
> >  		if (vcpu == VP_INVAL)
> >  			return false;
> > @@ -191,7 +198,7 @@ static bool __send_ipi_mask(const struct cpumask *mask, int vector)
> >  	return hv_result_success(status);
> > 
> >  do_ex_hypercall:
> > -	return __send_ipi_mask_ex(mask, vector);
> > +	return __send_ipi_mask_ex(mask, vector, exclude_self);
> >  }
> 
> This all looks correct to me, except for one difference compared with the
> current code.  In the current code, if the cpumask passed to
> hv_send_ipi_mask_allbutself() indicates only a single CPU that is "self",
> __send_ipi_mask() will detect that the cpumask is now empty, and 
> correctly return success without making the hypercall.  But
> the new code will make the hypercall with an empty input mask (both
> in the SEND_IPI and SEND_IPI_EX cases).   The Hyper-V TLFS is silent
> on whether such a hypercall is a no-op that returns success or is an
> error.  We'll have a problem if it is an error.  I think the safest thing
> is to enhance the cpumask_empty() test at the beginning of
> __send_ipi_mask() to also detect the case where only a single CPU
> is specified, and it is "self".   This could be done using cpumask_weight()
> and checking for zero as the "empty" case.   Then check for "1", and if
> exclude_self is set, check if it is the "self" CPU.

Sure. Making this change should not be too difficult.

Wei.