linux-kernel - Re: [PATCH 0/2] incorrect cpumask behavior with CPUMASK

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: <DDE943EC-3D5B-420D-A227-BF39C07963D8@linuxhacker.ru>
Date:	Fri, 27 Feb 2015 12:51:59 -0500
From:	Oleg Drokin <green@...uxhacker.ru>
To:	Rusty Russell <rusty@...tcorp.com.au>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	"David S. Miller" <davem@...emloft.net>,
	"<linux-kernel@...r.kernel.org> Mailing List" 
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 0/2] incorrect cpumask behavior with CPUMASK_OFFSTACK

Hello!

On Feb 27, 2015, at 6:46 AM, Rusty Russell wrote:

> green@...uxhacker.ru writes:
>> From: Oleg Drokin <green@...uxhacker.ru>
>> 
>> I just got a report today from Tyson Whitehead <twhitehead@...il.com>
>> that Lustre crashes when CPUMASK_OFFSTACK is enabled.
>> 
>> A little investigation revealed that this code:
>>        cpumask_t                       mask;
>> ...
>>        cpumask_copy(&mask, topology_thread_cpumask(0));
>>        weight = cpus_weight(mask);
> Yes.  cpumask_weight should have been used here.  The old cpus_* are
> deprecated.

Oh! I guess we missed the announcement.
I'll convert it over.

Should I also do a patch converting all other users and removing the deprecated
functions while I am at it?

>> The second patch that I am not sure if we wnat, but it seems to be useful
>> until struct cpumask is fully dynamic is to convert what looks like
>> whole-set operations e.g. copies, namely:
>> cpumask_setall, cpumask_clear, cpumask_copy to always operate on NR_CPUS
>> bits to ensure there's no stale garbage left in the mask should the
>> cpu count increases later.
> You can't do this, because dynamically allocated cpumasks don't have
> NR_CPUS bits.

Well, right now they certainly have. As in, it's a static define and we allocate
a bitmap to fit the (in my case) up to 8192 bits into such off-stack masks.

The concern is since number of cpus is not really a fixed variable, when you
do cpumask initialization, and then number of cpus grows, the end of the mask
could be garbage? Am I overthinking this and it's not really a problem?

> Let's just kill all the cpus_ functions.  This wasn't done originally
> because archs which didn't care about offline cpumasks didn't want the
> churn.  In particular, they must not copy struct cpumask by assignment,
> and fixing those is a fair bit of churn.

Ah, copy masks by assignment, I see.
Well, I guess we can eliminate the users outside of the affected arch trees
(I assume in x86 there would be no objections?) and perhaps add a warning to
checkpatch.pl?

Thanks!

Bye,
    Oleg--
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/