linux-kernel - Re: [PATCH 05/16] staging/lustre: fix up obsolete cpu function usage.

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:	Mon, 2 Mar 2015 20:16:23 -0500
From:	Oleg Drokin <green@...uxhacker.ru>
To:	Rusty Russell <rusty@...tcorp.com.au>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 05/16] staging/lustre: fix up obsolete cpu function usage.


On Mar 2, 2015, at 6:39 PM, Rusty Russell wrote:
>> if (i == NR_CPUS)
>>    blah;
>> 
>> when we are replacing for_each_cpu_mask with for_each_cpu,
>> what do we check the counter against now to see that the entire loop was executed
>> and we did not exit prematurely? nr_cpu_ids?
> You want >= nr_cpu_ids here.

Aha, Thanks!

>> Also I assume we still want to get rid of direct cpumask assignments like
>>> mask = *cpumask_of_node(cpu_to_node(index));
> 
> Yes, but this code is wrong anyway:
> 
> 		mask = *cpumask_of_node(cpu_to_node(index));
> 		for (i = max; i < num_online_cpus(); i++)
> 			cpumask_clear_cpu(i, &mask);
> 
> *Never* iterate to num_online_cpus().  eg. if cpus 0 and 3 are online,
> num_online_cpus() == 2.  I'm not sure what this code is doing, but it's
> not doing it well :)

Oh my, I don't know how I have not seen it sooner. I think I developed an
idea of what it is trying to do.
Thanks for highlighting it.

So there are 7 more users like this outside of Lustre in the kernel then, I'll try for a patch:
./drivers/scsi/hpsa.c:	for (i = 0; i < num_online_cpus(); i++) { -- this one seems to be just opencoding for_each_cpu, though

./arch/um/kernel/smp.c:	for (i = 0; i < num_online_cpus(); i++) { -- I wonder if UML is able to have discontiguous cpus up?

./arch/sh/include/asm/mmu_context.h:	for (i = 0; i < num_online_cpus(); i++) -- this seems to be the same bug we have.

./arch/sh/kernel/smp.c:		for (i = 0; i < num_online_cpus(); i++) -- this and the two below it too
./arch/sh/kernel/smp.c:		for (i = 0; i < num_online_cpus(); i++)
./arch/sh/kernel/smp.c:		for (i = 0; i < num_online_cpus(); i++)

./arch/m32r/kernel/smpboot.c:	for (cpu_id = 0 ; cpu_id < num_online_cpus() ; cpu_id++) -- this also is buggy, though it's just an info print.

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/