linux-kernel - Re: NUMA x86: add constraints check for nid parameters

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] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 6 Dec 2011 12:45:28 -0800 (PST)
From:	David Rientjes <rientjes@...gle.com>
To:	Petr Holasek <pholasek@...hat.com>
cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>, linux-kernel@...r.kernel.org,
	Anton Arapov <anton@...hat.com>
Subject: Re: NUMA x86: add constraints check for nid parameters

On Fri, 2 Dec 2011, Petr Holasek wrote:

> > > > > This patch adds constraints checks into __node_distance() and
> > > > > numa_set_distance() functions. If from or to parameters are
> > > > > lower than zero, it results into oops now.
> > > > 
> > > > Passing negative numbers into __node_distance() sounds like a bug in
> > > > the caller, and this patch will remove our means of detecting that bug.
> > > 
> > > That's true, but upper boundary is checked now, so why not to check lower?
> > 
> > Because it adds more code to the kernel and can hide bugs?
> > 

The upper bound is checked to ensure that we don't dereference past end of 
the array that stores the distance table, so it will catch errors for 
things like memory hotplug when additional nodes are onlined and the data 
structure isn't updated accordingly.

> > If what we're doing here is to be defensive against buggy BIOS tables
> > (a good idea) then we should validate the BIOS table values as close as
> > possible to the point where they were read frmo the BIOS.  And we should
> > (probably) emit a warning if a bad table entry is detected, rather than
> > silently fixing it up.
> 
> numa_set_distance() does exactly what you described above, only emits a
> warning. I agree with your objections with __node_distance() checks, it
> really can hide bugs in caller. So silent fix-up is the main problem and
> we shouldn't check anything so the caller will be advised when using 
> wrong nid by oops with a benefit of less code for us. Do I understand your
> opinion on this type of code?

I'd have no objection to adding a check to numa_set_distance() to ensure 
the node ids are non-negative in the same way we check that the distances 
themselves are non-negative; that can catch errors when pxms are used 
uninitialized when parsing the SRAT.  However, I think adding the check to 
__node_distance() is unnecessary.
--
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/