linux-kernel - Re: [RFC][PATCH v2] slub: Keep page and object in sync in slab_alloc

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, 21 Jan 2013 01:48:07 +0000
From:	Christoph Lameter <cl@...ux.com>
To:	Steven Rostedt <rostedt@...dmis.org>
cc:	LKML <linux-kernel@...r.kernel.org>, linux-mm <linux-mm@...ck.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Pekka Enberg <penberg@...nel.org>,
	Matt Mackall <mpm@...enic.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	RT <linux-rt-users@...r.kernel.org>,
	Clark Williams <clark@...hat.com>,
	John Kacur <jkacur@...il.com>,
	"Luis Claudio R. Goncalves" <lgoncalv@...hat.com>
Subject: Re: [RFC][PATCH v2] slub: Keep page and object in sync in
 slab_alloc_node()

On Fri, 18 Jan 2013, Steven Rostedt wrote:

> I'm curious to why not just add the preempt disable? It's rather quick
> and avoids all this complex trickery, which is just prone to bugs. It
> would make it much easier for others to review as well, and also keeps
> the setting of page, objects and cpu_slab consistent with everything
> else (which is assigned under preempt(irq)_disable).

Because this_cpu_read does not need the code to do a preempt disable on
x86 and on any other arch that will support this_cpu_read. this_cpu_read()
is implementable on many platform with a register  / offset in the same
way as on x86.

> > Well, the consequence would be that an object from another node than
> > desired will be allocated. Not that severe of an issue.
>
> Yes, it's not that severe of an issue, but it is still incorrect code.
> Why not just allocate on whatever node you want then? Why bother with
> the check at all?

The check so far has worked correctly for all tests.
Just because a rare race condition has been detected that may cause an
incorrect allocation does not mean that the check has no purpose at all.
And of course it needs to be fixed.

My patch with the check for page = NULL is enough to fix the potential
NULL pointer deref (which also is another case of a rare race that has
survived lots of tests so far).

The other issue with the wrong node needs some more thought and some tests
on the impact on the instruction overhead.

--
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/