linux-kernel - Re: [PATCH 2/4] x86: fix cpu_mask_to_apicid_and to include cpu_online

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]

Date:	Fri, 12 Dec 2008 08:37:06 -0800
From:	Mike Travis <travis@....com>
To:	Rusty Russell <rusty@...tcorp.com.au>
CC:	Ingo Molnar <mingo@...hat.com>, "H. Peter Anvin" <hpa@...or.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/4] x86: fix cpu_mask_to_apicid_and to include cpu_online_mask

Rusty Russell wrote:
> On Thursday 11 December 2008 21:58:08 Mike Travis wrote:
>> Impact: fix potential problem.
>>
>> In determining the destination apicid, there are usually three cpumasks
>> that are considered: the incoming cpumask arg, cfg->domain and the
>> cpu_online_mask.  Since we are just introducing the cpu_mask_to_apicid_and
>> function, make sure it includes the cpu_online_mask in it's evaluation.
> 
> Yerk.  Can we really "fail" cpu_mask_to_apicid_and with no repercussions?
> And does it make sense to try to fix this there?

Fail?  The only failure is if there is not a cpu that satisfies the conjunction
of the three masks, in which case it returns BAD_APICID.

The old procedure was to:

function(..., cpumask_t mask)
{
	cpumask_t tmp;

	cpus_and(tmp, mask, cfg->domain);
	...
	cpus_and(tmp, tmp, cpu_online_map);
	dest = cpu_mask_to_apicid(tmp);
	...
}

So making cpu_mask_to_apicid_and return:

	dest = cpu_mask_to_apicid(mask1 & mask2 & cpu_online_mask);

maintains this compatibility.

Turns out that there are two functions that did not AND all three masks,
but those two used TARGET_CPUS as one of the arguments.  All the x86
variations except NUMAQ, only return TARGET_CPUS which are online
(and that's probably a mistake in NUMAQ.)  So the above AND'ing is a
NOP in these cases and harmless.

> 
> This is not a new problem with the cpumask patches is it?  I toyed with a
> patch which converts flush_tlb_others, and it actually ensures that those
> cases never hand an offline mask to cpu_mask_to_apicid_and as a side
> effect (others still might).

No, I introduced the problem when I added cpu_mask_to_apicid_and(), and I
wanted to fix it before it was "officially" released.  I've been doing
benchmark testing with random HOTPLUG off and on events, and it's becoming
clear that setting the destination apicid to an off line cpu is definitely
a no-no. ;-)

Thanks,
Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/