linux-kernel - Re: v2.6.26-rc7: BUG: unable to handle kernel NULL pointer dereference

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:	Sun, 22 Jun 2008 18:29:07 +0200
From:	"Vegard Nossum" <vegard.nossum@...il.com>
To:	"Adrian Bunk" <bunk@...nel.org>
Cc:	"Rusty Russell" <rusty@...tcorp.com.au>,
	"Srivatsa Vaddagiri" <vatsa@...ibm.com>,
	"Mike Travis" <travis@....com>, linux-kernel@...r.kernel.org,
	"Gautham R Shenoy" <ego@...ibm.com>,
	"Rafael J. Wysocki" <rjw@...k.pl>,
	"Zhang, Yanmin" <yanmin.zhang@...el.com>,
	"Heiko Carstens" <heiko.carstens@...ibm.com>
Subject: Re: v2.6.26-rc7: BUG: unable to handle kernel NULL pointer dereference

On Sun, Jun 22, 2008 at 5:56 PM, Adrian Bunk <bunk@...nel.org> wrote:
>> commit e37d05dad7ff9744efd8ea95a70d389e9a65a6fc
>> Author: Mike Travis <travis@....com>
>> Date:   Thu May 1 04:35:16 2008 -0700
>>
>>     cpu: change cpu_sys_devices from array to per_cpu variable
>>
>>     Change cpu_sys_devices from array to per_cpu variable in drivers/base/cpu.c.
>>...
>
> Can you confirm whether this is definitely the cause or not?
>
> E.g. if it is and 2.6.25 works fine it might qualify as a 2.6.26-rc
> regression.

Hm, no. Each time I run this test, I get a different error (has been
NULL pointer, stuck CPU, circular locking dependency, ...) :-D

But if you look at the patch, I KNOW that this function is the one
that returns NULL, and it does so because the check is now stricter
than before. The hunk was:

-       if (cpu < NR_CPUS)
-               return cpu_sys_devices[cpu];
+       if (cpu < nr_cpu_ids && cpu_possible(cpu))
+               return per_cpu(cpu_sys_devices, cpu);
        else
                return NULL;

And the (cpu < nr_cpu_ids) fails because the CPU has just been
offlined (or failed to initialize, but it's the same thing), while
NR_CPUS is the value that was compiled in as CONFIG_NR_CPUS (so the
former check will always be true).

I don't think it is valid to ask for a per_cpu() variable on a CPU
which does not exist, though, so I don't know what the right fix would
be. A straight revert would be possible, but probably not desirable.
I'm so definitely not an expert in this area, but this "fix" _looks_
correct to me:

diff --git a/drivers/base/topology.c b/drivers/base/topology.c
index fdf4044..3bd95fd 100644
--- a/drivers/base/topology.c
+++ b/drivers/base/topology.c
@@ -143,14 +143,10 @@ static int __cpuinit topology_cpu_callback(struct notifier
        int rc = 0;

        switch (action) {
-       case CPU_UP_PREPARE:
-       case CPU_UP_PREPARE_FROZEN:
+       case CPU_ONLINE:
                rc = topology_add_dev(cpu);
                break;
-       case CPU_UP_CANCELED:
-       case CPU_UP_CANCELED_FROZEN:
-       case CPU_DEAD:
-       case CPU_DEAD_FROZEN:
+       case CPU_DOWN_PREPARE:
                topology_remove_dev(cpu);
                break;
        }

I'm sorry, I can't really say whether it's a regression or not. But
I'd bet it is.


Vegard

-- 
"The animistic metaphor of the bug that maliciously sneaked in while
the programmer was not looking is intellectually dishonest as it
disguises that the error is the programmer's own creation."
	-- E. W. Dijkstra, EWD1036
--
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/