Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 15 Nov 2016 16:08:13 -0800
From: Kees Cook <>
To: Michael Ellerman <>
Cc: Andrew Morton <>, Christoph Lameter <>, 
	Pekka Enberg <>, David Rientjes <>, 
	Joonsoo Kim <>, Linux-MM <>, 
	LKML <>, 
	"" <>

On Tue, Nov 15, 2016 at 3:50 PM, Michael Ellerman <> wrote:
> Kees Cook <> writes:
>> On Tue, Nov 15, 2016 at 2:57 AM, Michael Ellerman <> wrote:
>>> POISON_POINTER_DELTA is defined in poison.h, and is intended to be used
>>> to shift poison values so that they don't alias userspace.
>>> We should add it to ZERO_SIZE_PTR so that attackers can't use
>>> ZERO_SIZE_PTR as a way to get a pointer to userspace.
>> Ah, when dealing with a 0-sized malloc or similar?
> Yeah as returned by a 0-sized kmalloc for example.
>> Do you have pointers to exploits that rely on this?
> Not real ones, it was used in the StringIPC challenge:
> Though that included the ability to seek to an arbitrary offset from the
> zero size pointer, so this wouldn't have helped.
>> Regardless, normally PAN/SMAP-like things should be sufficient to
>> protect against this.
> True. Not everyone has PAN/SMAP though :)

Right, mostly just thinking out loud about the threat model and the
existing results.

>> Additionally, on everything but x86_64 and arm64, POISON_POINTER_DELTA
>> == 0, if I'm reading correctly:
> You are reading correctly. All 64-bit arches should be able to define it
> to something though.
>> Is the plan to add ILLEGAL_POINTER_VALUE for powerpc too?
> Yep. I should have CC'ed you on the patch :)

I suspected I was missing something. ;)

>> And either way, this patch, IIUC, will break the ZERO_OR_NULL_PTR()
>> check, since suddenly all of userspace will match it. (Though maybe
>> that's okay?)
> Yeah I wasn't sure what to do with that.

Yeah, though there are shockingly few callers of that macro. I think
building with HARDENED_USERCOPY would totally break the kernel,
though, since check_bogus_address() is looking at ZERO_OR_NULL even
for things destined for userspace.

> I don't think it breaks it, but it does become a bit fishy because as
> you say all of userspace (and more) will now match.
> It should probably just become two separate tests, though that
> potentially has issues with double evaluation of the argument. AFAICS
> none of the callers pass an expression though.

That shouldn't be a problem. I think we can use fancy magic like:

#define ZERO_OR_NULL_PTR(x) \
 ({ \
    unsigned long p = (unsigned long)(x); \
    (p == NULL || p == ZERO_SIZE_PTR); \

Though this technically loses the check for values 1 through 15...


Kees Cook
Nexus Security

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.