2009-06-02 15:37:17

by Christoph Lameter

[permalink] [raw]
Subject: Re: [PATCH] Change ZERO_SIZE_PTR to point at unmapped space

On Sat, 30 May 2009, Larry H. wrote:

> Let me provide you with a realistic scenario:
>
> 1. foo.c network protocol implementation takes a sockopt which
> sets some ACME_OPTLEN value taken from userland.
>
> 2. the length is not validated properly: it can be zero or an
> integer overflow / signedness issue allows it to wrap to zero.
>
> 3. kmalloc(0) ensues, and data is copied to the pointer
> returned. if this is the default ZERO_SIZE_PTR*, a malicious user
> can mmap a page at NULL, and read data leaked from kernel memory
> everytime that setsockopt is issued.
> (*: kmalloc of zero returns ZERO_SIZE_PTR)

Cannot happen. The page at 0L is not mapped. This will cause a fault.

You are assuming the system has already been breached. Then of course all
bets are off.

> The performance impact, if any, is completely negligible. The security
> benefits of this utterly simple change well surpass the downsides.

Dont see any security benefit. If there is a way to breach security
of the kernel via mmap then please tell us and then lets fix
the problem and not engage in dealing with secondary issues.

Semantics of mmap(NULL, ...) is that the kernel selects a valid address
for you. How are you mapping something at 0L?


2009-06-02 20:32:34

by Larry H.

[permalink] [raw]
Subject: Re: [PATCH] Change ZERO_SIZE_PTR to point at unmapped space

On 11:37 Tue 02 Jun , Christoph Lameter wrote:
> On Sat, 30 May 2009, Larry H. wrote:
>
> > Let me provide you with a realistic scenario:
> >
> > 1. foo.c network protocol implementation takes a sockopt which
> > sets some ACME_OPTLEN value taken from userland.
> >
> > 2. the length is not validated properly: it can be zero or an
> > integer overflow / signedness issue allows it to wrap to zero.
> >
> > 3. kmalloc(0) ensues, and data is copied to the pointer
> > returned. if this is the default ZERO_SIZE_PTR*, a malicious user
> > can mmap a page at NULL, and read data leaked from kernel memory
> > everytime that setsockopt is issued.
> > (*: kmalloc of zero returns ZERO_SIZE_PTR)
>
> Cannot happen. The page at 0L is not mapped. This will cause a fault.

Why would mmap_min_addr have been created in first place, if NULL can't
be mapped to force the kernel into accessing userland memory? This is
the way a long list of public and private kernel exploits have worked to
elevate privileges, and disable SELinux/LSMs atomically, too.

Take a look at these:
http://www.grsecurity.net/~spender/exploit.tgz (disables LSMs)
http://milw0rm.com/exploits/4172
http://milw0rm.com/exploits/3587

I would like to know what makes you think I can't mmap(0) from within
the same process that triggers your 'not so exploitable NULL page
fault', which instead of generating the oops will lead to 100% reliable,
cross-arch exploitation to get root privileges (again, after disabling
SELinux and anything else that would supposedly prevent this situation).
Or leaked memory, like a kmalloc(0) situation will most likely lead to,
given the current circumstances.

> You are assuming the system has already been breached. Then of course all
> bets are off.

No, your system has been breached and they have access as a
not-yet-privileged user. The bets are off when nothing protects your
kernel from letting them escalate privileges and disable your fancy
SELinux MLS policy, AppArmor, or any other LSM useless in this scenario.

> > The performance impact, if any, is completely negligible. The security
> > benefits of this utterly simple change well surpass the downsides.
>
> Dont see any security benefit. If there is a way to breach security
> of the kernel via mmap then please tell us and then lets fix
> the problem and not engage in dealing with secondary issues.

Your first concern has been addressed above. Regarding the second, well,
this is called proactive defense. Instead of taking a reactive approach
when your security has been already breached, you try to lock down
potential venues of attack to deter unknown threats.

Instead of the definitive tone and so forth, you could try something more
reasonable like 'I do not understand what this is all about, could you
please explain it?', which might help.

> Semantics of mmap(NULL, ...) is that the kernel selects a valid address
> for you. How are you mapping something at 0L?

http://www.opengroup.org/onlinepubs/000095399/functions/mmap.html

Please proceed to re-read the part about anonymous mappings and
MAP_FIXED|MAP_PRIVATE. And refer to the exploits mentioned in the
previous paragraphs ;)

Once mmap semantics are clear, we can continue discussing any other
possible objections to this patch, if you don't mind.

Larry

(Please keep pageexec/PaX team in CC)

2009-06-03 14:51:06

by Christoph Lameter

[permalink] [raw]
Subject: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

On Tue, 2 Jun 2009, Larry H. wrote:

> Why would mmap_min_addr have been created in first place, if NULL can't
> be mapped to force the kernel into accessing userland memory? This is
> the way a long list of public and private kernel exploits have worked to
> elevate privileges, and disable SELinux/LSMs atomically, too.
>
> Take a look at these:
> http://www.grsecurity.net/~spender/exploit.tgz (disables LSMs)
> http://milw0rm.com/exploits/4172
> http://milw0rm.com/exploits/3587
>
> I would like to know what makes you think I can't mmap(0) from within
> the same process that triggers your 'not so exploitable NULL page
> fault', which instead of generating the oops will lead to 100% reliable,
> cross-arch exploitation to get root privileges (again, after disabling
> SELinux and anything else that would supposedly prevent this situation).
> Or leaked memory, like a kmalloc(0) situation will most likely lead to,
> given the current circumstances.

Ok. So what we need to do is stop this toying around with remapping of
page 0. The following patch contains a fix and a test program that
demonstrates the issue.


Subject: [Security] Do not allow remapping of page 0 via MAP_FIXED

If one remaps page 0 then the kernel checks for NULL pointers of various
flavors are bypassed and this may be exploited in various creative ways
to transfer data from kernel space to user space.

Fix this by not allowing the remapping of page 0. Return -EINVAL if
such a mapping is attempted.

Simple test program that shows the problem:

#include <sys/mman.h>

int main(int argc, char *argv)
{
printf("%ld\n", mmap(0L, 4096, PROT_WRITE, MAP_FIXED|MAP_PRIVATE|MAP_ANONYMOUS, 0,0));
*((char *)8) = 3;
printf("Value at address 8 is %d\n", *((char *)8));
return 0;
}

If the remapping of page 0 succeeds then the value at 8 is 3.
After the patch the program segfaults as it should.

Signed-off-by: Christoph Lameter <[email protected]>

---
mm/mmap.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)

Index: linux-2.6/mm/mmap.c
===================================================================
--- linux-2.6.orig/mm/mmap.c 2009-06-03 09:44:43.000000000 -0500
+++ linux-2.6/mm/mmap.c 2009-06-03 09:45:31.000000000 -0500
@@ -1273,8 +1273,12 @@ arch_get_unmapped_area(struct file *filp
if (len > TASK_SIZE)
return -ENOMEM;

- if (flags & MAP_FIXED)
- return addr;
+ if (flags & MAP_FIXED) {
+ if (addr & PAGE_MASK)
+ return addr;
+ /* Do not allow remapping of the first page */
+ return -EINVAL;
+ }

if (addr) {
addr = PAGE_ALIGN(addr);
@@ -1349,8 +1353,12 @@ arch_get_unmapped_area_topdown(struct fi
if (len > TASK_SIZE)
return -ENOMEM;

- if (flags & MAP_FIXED)
- return addr;
+ if (flags & MAP_FIXED) {
+ if (addr & PAGE_MASK)
+ return addr;
+ /* Do not allow remapping of the first page */
+ return -EINVAL;
+ }

/* requesting a specific address */
if (addr) {

2009-06-03 15:08:44

by Linus Torvalds

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)



On Wed, 3 Jun 2009, Christoph Lameter wrote:
>
> Ok. So what we need to do is stop this toying around with remapping of
> page 0. The following patch contains a fix and a test program that
> demonstrates the issue.

No, we _need_ to be able to map to address zero.

It may not be very common, but things like vm86 require it - vm86 mode
always starts at virtual address zero.

For similar reasons, some other emulation environments will want it too,
simply because they want to emulate another environment that has an
address space starting at 0, and don't want to add a base to all address
calculations.

There are historically even some crazy optimizing compilers that decided
that they need to be able to optimize accesses of a pointer across a NULL
pointer check, so that they can turn code like

if (!ptr)
return;
val = ptr->member;

into doing the load early. In order to support that optimization, they
have a runtime that always maps some garbage at virtual address zero.

(I don't remember who did this, but my dim memory wants to say it was some
HP-UX compiler. Scheduling loads early can be a big deal on especially
in-order machines with nonblocking cache accesses).

The point being that we do need to support mmap at zero. Not necessarily
universally, but it can't be some fixed "we don't allow that".

Linus

2009-06-03 15:20:48

by Stephen Smalley

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

On Wed, 2009-06-03 at 10:50 -0400, Christoph Lameter wrote:
> On Tue, 2 Jun 2009, Larry H. wrote:
>
> > Why would mmap_min_addr have been created in first place, if NULL can't
> > be mapped to force the kernel into accessing userland memory? This is
> > the way a long list of public and private kernel exploits have worked to
> > elevate privileges, and disable SELinux/LSMs atomically, too.
> >
> > Take a look at these:
> > http://www.grsecurity.net/~spender/exploit.tgz (disables LSMs)
> > http://milw0rm.com/exploits/4172
> > http://milw0rm.com/exploits/3587
> >
> > I would like to know what makes you think I can't mmap(0) from within
> > the same process that triggers your 'not so exploitable NULL page
> > fault', which instead of generating the oops will lead to 100% reliable,
> > cross-arch exploitation to get root privileges (again, after disabling
> > SELinux and anything else that would supposedly prevent this situation).
> > Or leaked memory, like a kmalloc(0) situation will most likely lead to,
> > given the current circumstances.
>
> Ok. So what we need to do is stop this toying around with remapping of
> page 0. The following patch contains a fix and a test program that
> demonstrates the issue.
>
>
> Subject: [Security] Do not allow remapping of page 0 via MAP_FIXED
>
> If one remaps page 0 then the kernel checks for NULL pointers of various
> flavors are bypassed and this may be exploited in various creative ways
> to transfer data from kernel space to user space.
>
> Fix this by not allowing the remapping of page 0. Return -EINVAL if
> such a mapping is attempted.

You can already prevent unauthorized processes from mapping low memory
via the existing mmap_min_addr setting, configurable via
SECURITY_DEFAULT_MMAP_MIN_ADDR or /proc/sys/vm/mmap_min_addr. Then
cap_file_mmap() or selinux_file_mmap() will apply a check when a process
attempts to map memory below that address.

--
Stephen Smalley
National Security Agency

2009-06-03 15:24:03

by Christoph Lameter

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

On Wed, 3 Jun 2009, Linus Torvalds wrote:

> The point being that we do need to support mmap at zero. Not necessarily
> universally, but it can't be some fixed "we don't allow that".

Hmmm... Depend on some capability? CAP_SYS_PTRACE may be something
remotely related?

2009-06-03 15:40:15

by Linus Torvalds

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)



On Wed, 3 Jun 2009, Christoph Lameter wrote:

> On Wed, 3 Jun 2009, Linus Torvalds wrote:
>
> > The point being that we do need to support mmap at zero. Not necessarily
> > universally, but it can't be some fixed "we don't allow that".
>
> Hmmm... Depend on some capability? CAP_SYS_PTRACE may be something
> remotely related?

But as mentioned several times, we do have the system-wide setting in
'mmap_min_addr' (that then can be overridden by CAP_SYS_RAWIO, so in that
sense a capability already exists).

It defaults to 64kB in at least the x86 defconfig files, but to 0 in the
Kconfig defaults. Also, for some reason it has a "depends on SECURITY",
which means that if you just default to the old-style unix security you'll
lose it.

So there are several ways to disable it by mistake. I don't know what
distros do.

Linus

2009-06-03 15:41:40

by Christoph Lameter

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

On Wed, 3 Jun 2009, Stephen Smalley wrote:

> > If one remaps page 0 then the kernel checks for NULL pointers of various
> > flavors are bypassed and this may be exploited in various creative ways
> > to transfer data from kernel space to user space.
> >
> > Fix this by not allowing the remapping of page 0. Return -EINVAL if
> > such a mapping is attempted.
>
> You can already prevent unauthorized processes from mapping low memory
> via the existing mmap_min_addr setting, configurable via
> SECURITY_DEFAULT_MMAP_MIN_ADDR or /proc/sys/vm/mmap_min_addr. Then
> cap_file_mmap() or selinux_file_mmap() will apply a check when a process
> attempts to map memory below that address.

mmap_min_addr depends on CONFIG_SECURITY which establishes various
strangely complex "security models".

The system needs to be secure by default.

2009-06-03 16:14:30

by Alan

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

> It defaults to 64kB in at least the x86 defconfig files, but to 0 in the
> Kconfig defaults. Also, for some reason it has a "depends on SECURITY",
> which means that if you just default to the old-style unix security you'll
> lose it.
>
> So there are several ways to disable it by mistake. I don't know what
> distros do.

Fedora at least uses SELinux to manage it. You need some kind of security
policy engine running as a few apps really need to map low space (mostly
for vm86)

2009-06-03 16:20:20

by Linus Torvalds

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)



On Wed, 3 Jun 2009, Christoph Lameter wrote:
>
> mmap_min_addr depends on CONFIG_SECURITY which establishes various
> strangely complex "security models".
>
> The system needs to be secure by default.

It _is_ secure by default. You have to do some pretty non-default things
to get away from it.

But I do agree that it might be good to move the thing into the generic
path. I just don't think your arguments are very good. It's not about
defaults, it's about the fact that this isn't worth being hidden by that
security layer.

Linus

2009-06-03 16:22:13

by Linus Torvalds

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)



On Wed, 3 Jun 2009, Alan Cox wrote:
>
> Fedora at least uses SELinux to manage it. You need some kind of security
> policy engine running as a few apps really need to map low space (mostly
> for vm86)

Well, vm86 isn't even an issue on x86-64, so it's arguable that at least a
few cases could very easily just make it more static and obvious.

Linus

2009-06-03 16:22:25

by Eric Paris

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

On Wed, Jun 3, 2009 at 11:38 AM, Linus Torvalds
<[email protected]> wrote:
>
>
> On Wed, 3 Jun 2009, Christoph Lameter wrote:
>
>> On Wed, 3 Jun 2009, Linus Torvalds wrote:
>>
>> > The point being that we do need to support mmap at zero. Not necessarily
>> > universally, but it can't be some fixed "we don't allow that".
>>
>> Hmmm... Depend on some capability? CAP_SYS_PTRACE may be something
>> remotely related?
>
> But as mentioned several times, we do have the system-wide setting in
> 'mmap_min_addr' (that then can be overridden by CAP_SYS_RAWIO, so in that
> sense a capability already exists).
>
> It defaults to 64kB in at least the x86 defconfig files, but to 0 in the
> Kconfig defaults. Also, for some reason it has a "depends on SECURITY",
> which means that if you just default to the old-style unix security you'll
> lose it.
>
> So there are several ways to disable it by mistake. I don't know what
> distros do.

Fedora has it on.

As I recall the only need for CONFIG_SECURITY is for the ability to
override the check.

I think I could probably pretty cleanly change it to use
CAP_SYS_RAWIO/SELinux permissions if CONFIG_SECURITY and just allow it
for uid=0 in the non-security case? Deny it for everyone in the
non-security case and make them change the /proc tunable if they need
it?

-Eric

2009-06-03 16:24:16

by Eric Paris

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

On Wed, Jun 3, 2009 at 12:19 PM, Linus Torvalds
<[email protected]> wrote:
>
>
> On Wed, 3 Jun 2009, Alan Cox wrote:
>>
>> Fedora at least uses SELinux to manage it. You need some kind of security
>> policy engine running as a few apps really need to map low space (mostly
>> for vm86)
>
> Well, vm86 isn't even an issue on x86-64, so it's arguable that at least a
> few cases could very easily just make it more static and obvious.

Wine does/did also use a zero page, can't remember what they used it
for off hand but they were mad at me when I added this....

-Eric

2009-06-03 16:26:56

by Larry H.

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

On 11:41 Wed 03 Jun , Christoph Lameter wrote:
> On Wed, 3 Jun 2009, Stephen Smalley wrote:
>
> > > If one remaps page 0 then the kernel checks for NULL pointers of various
> > > flavors are bypassed and this may be exploited in various creative ways
> > > to transfer data from kernel space to user space.
> > >
> > > Fix this by not allowing the remapping of page 0. Return -EINVAL if
> > > such a mapping is attempted.

Christopher, crippling the system is truly not the way to fix this.
There are many legitimate users of private|fixed mappings at 0. In
addition, if you want to go ahead and break POSIX, at least make sure
your patch closes the loophole.

Given these circumstances, are you proposing this over my patch?

Linus already pointed out the main (functional) problem about it. It
seems you are also confusing the issue, albeit already realized it can
be a venue of attack, which is good.

For instance, there are many scenarios in which a fixed mapping can be
used in a non-zero address to abuse kernel flaws... your patch is
useless against those.

Please let me remind you that my original intent was to prevent
kmalloc(0) from leading to potential NULL or offset-from-NULL access
issues, and not deterring NULL pointer deferences in kernel-land which
is a whole different thing (see PaX UDEREF for clues on this).

> >
> > You can already prevent unauthorized processes from mapping low memory
> > via the existing mmap_min_addr setting, configurable via
> > SECURITY_DEFAULT_MMAP_MIN_ADDR or /proc/sys/vm/mmap_min_addr. Then
> > cap_file_mmap() or selinux_file_mmap() will apply a check when a process
> > attempts to map memory below that address.

If SELinux isn't present, that's not useful. If mmap_min_addr is
enabled, that still won't solve what my original, utterly simple patch
fixes.

The patch provides a no-impact, clean solution to prevent kmalloc(0)
situations from becoming a security hazard. Nothing else.

If you want to solve NULL/ptr deference abuse from userland, you better
start thinking about separating kernel virtual address space from
userland's, with the performance impact that implies. Few architectures
provide this capability without performance hit, and x86 ain't one of
them.

> mmap_min_addr depends on CONFIG_SECURITY which establishes various
> strangely complex "security models".
>
> The system needs to be secure by default.

Correct, so what was wrong with my patch again? That the original two
line change was written by the PaX team?

Come on chap, It's not like you will lose your bragging rights among
your peers for admitting that I was right. Just this one time. I won't
tell anybody. Promise.

Larry

2009-06-03 16:29:59

by Linus Torvalds

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)



On Wed, 3 Jun 2009, Eric Paris wrote:
>
> As I recall the only need for CONFIG_SECURITY is for the ability to
> override the check.

No, if you have SECURITY disabled entirely, the check goes away.

If you have SECURITY on, but then use the simple capability model, the
check is there.

If you have SECURITY on, and then use SElinux, you can make it be dynamic.

> I think I could probably pretty cleanly change it to use
> CAP_SYS_RAWIO/SELinux permissions if CONFIG_SECURITY and just allow it
> for uid=0 in the non-security case?

We probably should, since the "capability" security version should
generally essentially emulate the regular non-SECURITY case for root.

Linus

2009-06-03 16:32:18

by Eric Paris

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

On Wed, Jun 3, 2009 at 12:28 PM, Linus Torvalds
<[email protected]> wrote:
>
>
> On Wed, 3 Jun 2009, Eric Paris wrote:
>>
>> As I recall the only need for CONFIG_SECURITY is for the ability to
>> override the check.
>
> No, if you have SECURITY disabled entirely, the check goes away.

I meant 'need' as in the reason I wrapped it in CONFIG_SECURITY, not
that you were wrong when you said it disapeared.

>> I think I could probably pretty cleanly change it to use
>> CAP_SYS_RAWIO/SELinux permissions if CONFIG_SECURITY and just allow it
>> for uid=0 in the non-security case?
>
> We probably should, since the "capability" security version should
> generally essentially emulate the regular non-SECURITY case for root.

Will poke/patch this afternoon.

-Eric

2009-06-03 16:37:42

by Rik van Riel

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

Larry H. wrote:

> Christopher, crippling the system is truly not the way to fix this.
> There are many legitimate users of private|fixed mappings at 0. In
> addition, if you want to go ahead and break POSIX, at least make sure
> your patch closes the loophole.

I suspect there aren't many at all, and restricting them through
SELinux may be enough to mitigate the risk.

> If SELinux isn't present, that's not useful. If mmap_min_addr is
> enabled, that still won't solve what my original, utterly simple patch
> fixes.

Would anybody paranoid run their system without SELinux?

> The patch provides a no-impact, clean solution to prevent kmalloc(0)
> situations from becoming a security hazard. Nothing else.

True, the changes in your patch only affect a few code paths.

--
All rights reversed.

2009-06-03 16:45:27

by Linus Torvalds

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)



On Wed, 3 Jun 2009, Eric Paris wrote:
> >
> > We probably should, since the "capability" security version should
> > generally essentially emulate the regular non-SECURITY case for root.
>
> Will poke/patch this afternoon.

Btw, a perhaps more interesting case would be to _really_ make the
"!SECURITY" case just be essentially a hardcoding of the capability case.

Right now the Kconfig option is actually actively misleading, because it
says that "if you don't enable SECURITY, the default security model will
be used".

And that's not automatically true, as shown by this example. You can
easily get out of sync between security/capability.c and the hardcoded
non-security rules in include/linux/security.h.

Wouldn't it be kind of nice if the "security/capability.c" file would work
something like

- make the meat of it just a header file ("<linux/cap_security.h>")

- if !SECURITY, the functions become inline functions named
"security_xyz()", and the header file gets included from
<linux/security.h>

- if SECURITY, the functions become static functions named "cap_xyz()",
and get included from security/capability.c.

IOW, we'd _guarantee_ that the !SECURITY case is exactly the same as the
SECURITY+default capabilities case, because we'd be sharing the source
code.

Hmm? Wouldn't that be a nice way to always avoid the potential "oops,
!SECURITY has different semantics than intended".

Linus

2009-06-03 16:48:20

by Linus Torvalds

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)



On Wed, 3 Jun 2009, Rik van Riel wrote:
>
> Would anybody paranoid run their system without SELinux?

You make two very fundamental mistakes.

The first is to assume that this is about "paranoid" people. Security is
_not_ about people who care deeply about security. It's about everybody.
Look at viruses and DDoS attacks - the "paranoid" people absolutely depend
on the _non_paranoid people being secure too!

The other mistake is to think that SELinux is sane, or should be the
default. It's a f*cking complex disaster, and makes performance plummet on
some things. I turn it off, and I know lots of other sane people do too.
So the !SElinux case really does need to work.

Linus

2009-06-03 17:16:22

by Eric Paris

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

On Wed, Jun 3, 2009 at 12:47 PM, Linus Torvalds
<[email protected]> wrote:

> The other mistake is to think that SELinux is sane, or should be the
> default. It's a f*cking complex disaster, and makes performance plummet on
> some things.

While I think you couldn't be more wrong I'm not going to argue that topic....

I am at least interested in hearing about the 'performance plummet.'
I don't see any performance reports on my todo list but I am
interested in banging on any that people report. Last performance
thing I heard anything about was Ingo doing some profiling of of a
network stack benchmark in which SELinux was eating a percent or two
his time. I cut the SELinux performance penalty by about 50% on my
systems in that benchmark. If others have complaints let me or the
selinux list know....

-Eric

2009-06-03 17:19:43

by Larry H.

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

On 12:36 Wed 03 Jun , Rik van Riel wrote:
> Larry H. wrote:
>
>> Christopher, crippling the system is truly not the way to fix this.
>> There are many legitimate users of private|fixed mappings at 0. In
>> addition, if you want to go ahead and break POSIX, at least make sure
>> your patch closes the loophole.
>
> I suspect there aren't many at all, and restricting them through
> SELinux may be enough to mitigate the risk.

It's still perfectly valid POSIX, but I'm definitely keen on using this
patch together with a convenient mmap_min_addr value. I'm just trying to
show how both things are orthogonal to each other, without additional
cost for us (as in people doing kernel/drivers development) and users.

>> If SELinux isn't present, that's not useful. If mmap_min_addr is
>> enabled, that still won't solve what my original, utterly simple patch
>> fixes.
>
> Would anybody paranoid run their system without SELinux?

Does everyone who is conscious about security must use SELinux? Is
SELinux the only acceptable solution? What about people who decide to
use AppArmor, or LIDS, or grsecurity?

That's not a valid point. People should stay safe without SELinux
whenever it is feasible, IMHO. I think everyone here will agree that
SELinux has a track of being disabled by users after installation
because they don't want to invest the necessary time on understanding
and learning the policy language or management tools.

>> The patch provides a no-impact, clean solution to prevent kmalloc(0)
>> situations from becoming a security hazard. Nothing else.
>
> True, the changes in your patch only affect a few code paths.

Only SLAB code itself is affected, users of kmalloc won't see a
functional difference. They just won't be as easily abused if a zero
length ends up passed to kmalloc and the pointer is used for something
later. There's an issue here that I must note: a wraparound can happen
and make the pointer land back somewhere near NULL.

It could be changed to point at the start of the fixmap.

It might be wise to see if expanding the fixmap on runtime can deter
this, although I had trouble using it within vm guests. This can be done
using reservetop boot option.

Larry

2009-06-03 17:23:03

by Larry H.

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

On 09:47 Wed 03 Jun , Linus Torvalds wrote:
>
>
> On Wed, 3 Jun 2009, Rik van Riel wrote:
> >
> > Would anybody paranoid run their system without SELinux?
>
> You make two very fundamental mistakes.
>
> The first is to assume that this is about "paranoid" people. Security is
> _not_ about people who care deeply about security. It's about everybody.
> Look at viruses and DDoS attacks - the "paranoid" people absolutely depend
> on the _non_paranoid people being secure too!
>
> The other mistake is to think that SELinux is sane, or should be the
> default. It's a f*cking complex disaster, and makes performance plummet on
> some things. I turn it off, and I know lots of other sane people do too.
> So the !SElinux case really does need to work.

I'm finally glad we start finding points where we both agree. riel is
talking from the perspective of someone who deals with RHEL/Fedora... so
I could see his inclination towards SELinux over any other
possibilities.

But people without SELinux must be definitely taken care of, and kept
safe whenever possible, if technical circumstances allow this to happen.

Larry

2009-06-03 17:28:56

by Linus Torvalds

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)



On Wed, 3 Jun 2009, Eric Paris wrote:
>
> I am at least interested in hearing about the 'performance plummet.'

It's perhaps not so much SElinux itself, but the AUDIT support (which it
requires) that is really _very_ noticeable on microbenchmarks.

Last time I ran lmbench on a Fedora kernel it was horrible. Turning off
AUDIT (which also turns off SElinux) fixes it.

It may be crazy distro auditing rules or whatever, but that doesn't change
the basic issue.

Linus

2009-06-03 17:29:31

by Alan

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

> Ok. So what we need to do is stop this toying around with remapping of
> page 0. The following patch contains a fix and a test program that
> demonstrates the issue.

NAK - you've now broken half a dozen apps.

One way you could approach this would be to write a security module for
non SELINUX users - one that did one thing alone - decide whether the app
being run was permitted to map the low 64K perhaps by checking the
security label on the file.

Alan

2009-06-03 17:32:06

by Eric Paris

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

On Wed, Jun 3, 2009 at 1:28 PM, Linus Torvalds
<[email protected]> wrote:
>
>
> On Wed, 3 Jun 2009, Eric Paris wrote:
>>
>> I am at least interested in hearing about the 'performance plummet.'
>
> It's perhaps not so much SElinux itself, but the AUDIT support (which it
> requires) that is really _very_ noticeable on microbenchmarks.
>
> Last time I ran lmbench on a Fedora kernel it was horrible. Turning off
> AUDIT (which also turns off SElinux) fixes it.
>
> It may be crazy distro auditing rules or whatever, but that doesn't change
> the basic issue.

Probably AUDITSYSCALL, not AUDIT. SELinux only needs AUDIT. I'll
poke that too someday, thanks.

-Eric

2009-06-03 17:36:58

by Linus Torvalds

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)



On Wed, 3 Jun 2009, Alan Cox wrote:
>
> One way you could approach this would be to write a security module for
> non SELINUX users - one that did one thing alone - decide whether the app
> being run was permitted to map the low 64K perhaps by checking the
> security label on the file.

Unnecessary. I really think that 99% of all people are perfectly fine with
just the "mmap_min_addr" rule, and no more.

The rest could just use SElinux or set it to zero. It's not like allowing
mmap's at NULL is a huge problem. Sure, it allows a certain kind of attack
vector, but it's by no means an easy or common one - you need to already
have gotten fairly good local access to take advantage of it.

Linus

2009-06-03 17:58:57

by Larry H.

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

On 10:35 Wed 03 Jun , Linus Torvalds wrote:
>
>
> On Wed, 3 Jun 2009, Alan Cox wrote:
> >
> > One way you could approach this would be to write a security module for
> > non SELINUX users - one that did one thing alone - decide whether the app
> > being run was permitted to map the low 64K perhaps by checking the
> > security label on the file.
>
> Unnecessary. I really think that 99% of all people are perfectly fine with
> just the "mmap_min_addr" rule, and no more.
>
> The rest could just use SElinux or set it to zero. It's not like allowing
> mmap's at NULL is a huge problem. Sure, it allows a certain kind of attack
> vector, but it's by no means an easy or common one - you need to already
> have gotten fairly good local access to take advantage of it.

Are you saying that a kernel exploit can't be leveraged by means of
runtime code injection for example? By exploiting a completely
unprivileged daemon remotely? It's not 1990 anymore. People compromise
your system from memory, disk doesn't get to see the ladies poledancing
your kernel $pc.

Not easy? You should definitely ask the people who wrote those exploits what
kind of difficulty they encountered writing them. It goes like this:

1) Have kernel flaw which leads to function ptr call from NULL
or offset from NULL. Or can overwrite one. Or corrupt a kernel
object. Not challenging.

2) In your userland process, map NULL. Insert fake structures
with proper pointers to your shellcode of choice. Not
challenging.

3) Run the exploit.

Not common? Compile a list of past reported NULL ptr deference oopses
from the Red Hat bugzilla, kernel bugzilla or the LKML. Check how many
of those could be triggered via normal syscall/unprivileged code paths.

You didn't answer my follow-up to your initial mail arguing the patch
was of no use, where I described a realistic scenario. Does that mean
you agree with it? If not, I would like to hear your opinion.

Larry

2009-06-03 18:13:38

by Linus Torvalds

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)



On Wed, 3 Jun 2009, Larry H. wrote:
>
> Are you saying that a kernel exploit can't be leveraged by means of
> runtime code injection for example?

No. I'm sayng that sane people don't get hung up about every little
possibility.

Why are security people always so damn black-and-white? In most other
areas, such people are called "crazy" or "stupid", but the security people
seem to call them "normal".

The fact, the NULL pointer attack is neither easy nor common. It's
perfectly reasonable to say "we'll allow mmap at virtual address zero".

Disallowing NULL pointer mmap's is one small tool in your toolchest, and
not at all all-consumingly important or fundamental. It's just one more
detail.

Get over it. Don't expect everybody to be as extremist as you apparently
are.

Linus

2009-06-03 18:37:58

by Larry H.

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

On 11:12 Wed 03 Jun , Linus Torvalds wrote:
>
>
> On Wed, 3 Jun 2009, Larry H. wrote:
> >
> > Are you saying that a kernel exploit can't be leveraged by means of
> > runtime code injection for example?
>
> No. I'm sayng that sane people don't get hung up about every little
> possibility.

Nothing of what has been mentioned is a little possibility. Far from it.

> Why are security people always so damn black-and-white? In most other
> areas, such people are called "crazy" or "stupid", but the security people
> seem to call them "normal".

Security people? I honestly share some of the opinions on the industry
that you might have, and I'm likely taking a gamble stating this
publicly. You are right, there are bleeding imbeciles there. I'm not
part of that 'security people', and will never consider me one. My
interest on security, long time ago, started at the defensive side. I've
been doing kernel development for almost 5 years focusing on
developing _solutions_, not problems. Understanding the offensive side
in depth is a necessity if you want to be realistic on the defensive
one.

If I can help you understand it, and other kernel developers, to come to
a point where realistic, effective security improvements are deployed in
the kernel, we will have accomplished the one and only goal I had when I
started talking to riel or proposing patches.

>
> The fact, the NULL pointer attack is neither easy nor common. It's
> perfectly reasonable to say "we'll allow mmap at virtual address zero".

And how could you calibrate if this attack venue isn't easy to take
advantage of? Or not commonly abused? What empirical results led you to this
conclusion?

> Disallowing NULL pointer mmap's is one small tool in your toolchest, and
> not at all all-consumingly important or fundamental. It's just one more
> detail.

Definitely, another layer, part of a complex set of measures to deter
different kinds of flaws from all feasible sides. Fundamental to avoid
the situation it is designed to prevent. In the same way as changing
some pointers in the kernel for preventing unwise values to be used when
returning from 'unusual' code paths (kmalloc(0) for example).

> Get over it. Don't expect everybody to be as extremist as you apparently
> are.

Extremism is the new buzzword. What's next? I'm not the one following up
with dogmatic responses lacking any reasoning. I've supported my claims
every time I expressed them so far. And when I was mistaken or agreed
with a different opinion, I made it clear as well.

Sorry, Linus, but you are taking a long shot calling people names.

Larry

2009-06-03 18:50:45

by Linus Torvalds

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)



On Wed, 3 Jun 2009, Larry H. wrote:
> >
> > The fact, the NULL pointer attack is neither easy nor common. It's
> > perfectly reasonable to say "we'll allow mmap at virtual address zero".
>
> And how could you calibrate if this attack venue isn't easy to take
> advantage of? Or not commonly abused? What empirical results led you to this
> conclusion?

It's not a primary attack vector. You need to have already broken local
security to get there - you need to be able to execute code.

That means that you've already by-passed all the main security. It's thus
by definition less common than attack vectors like buffer overflows that
give you that capability in the first place.

Linus

2009-06-03 18:52:03

by Linus Torvalds

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)



On Wed, 3 Jun 2009, Linus Torvalds wrote:
>
> That means that you've already by-passed all the main security. It's thus
> by definition less common than attack vectors like buffer overflows that
> give you that capability in the first place.

Btw, you obviously need to then _also_ pair it with some as-yet-unknown
case of kernel bug to get to that NULL pointer (or zero-sized-alloc
pointer) problem.

You _also_ seem to be totally ignoring the fact that we already _do_
protect against NULL pointers by default.

So I really don't see why you're making a big deal of this. It's as if you
were talking about us not randomizing the address space - sure, you can
turn it off, but so what? We do it by default.

So it boils down to:

- NULL pointers already cannot be in mmap memory (unless a distro has
done something wrong - outside of the kernel)

- What's your beef? Let it go, man.

Linus

2009-06-03 19:00:08

by Christoph Lameter

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

We could just move the check for mmap_min_addr out from
CONFIG_SECURITY?


Use mmap_min_addr indepedently of security models

This patch removes the dependency of mmap_min_addr on CONFIG_SECURITY.
It also sets a default mmap_min_addr of 4096.

mmapping of addresses below 4096 will only be possible for processes
with CAP_SYS_RAWIO.


Signed-off-by: Christoph Lameter <[email protected]>

---
include/linux/mm.h | 2 --
kernel/sysctl.c | 2 --
mm/Kconfig | 19 +++++++++++++++++++
mm/mmap.c | 6 ++++++
security/Kconfig | 20 --------------------
security/capability.c | 2 --
security/security.c | 3 ---
7 files changed, 25 insertions(+), 29 deletions(-)

Index: linux-2.6/include/linux/mm.h
===================================================================
--- linux-2.6.orig/include/linux/mm.h 2009-06-03 13:48:01.000000000 -0500
+++ linux-2.6/include/linux/mm.h 2009-06-03 13:48:10.000000000 -0500
@@ -580,12 +580,10 @@ static inline void set_page_links(struct
*/
static inline unsigned long round_hint_to_min(unsigned long hint)
{
-#ifdef CONFIG_SECURITY
hint &= PAGE_MASK;
if (((void *)hint != NULL) &&
(hint < mmap_min_addr))
return PAGE_ALIGN(mmap_min_addr);
-#endif
return hint;
}

Index: linux-2.6/kernel/sysctl.c
===================================================================
--- linux-2.6.orig/kernel/sysctl.c 2009-06-03 13:48:01.000000000 -0500
+++ linux-2.6/kernel/sysctl.c 2009-06-03 13:48:10.000000000 -0500
@@ -1225,7 +1225,6 @@ static struct ctl_table vm_table[] = {
.strategy = &sysctl_jiffies,
},
#endif
-#ifdef CONFIG_SECURITY
{
.ctl_name = CTL_UNNUMBERED,
.procname = "mmap_min_addr",
@@ -1234,7 +1233,6 @@ static struct ctl_table vm_table[] = {
.mode = 0644,
.proc_handler = &proc_doulongvec_minmax,
},
-#endif
#ifdef CONFIG_NUMA
{
.ctl_name = CTL_UNNUMBERED,
Index: linux-2.6/mm/mmap.c
===================================================================
--- linux-2.6.orig/mm/mmap.c 2009-06-03 13:48:01.000000000 -0500
+++ linux-2.6/mm/mmap.c 2009-06-03 13:48:10.000000000 -0500
@@ -87,6 +87,9 @@ int sysctl_overcommit_ratio = 50; /* def
int sysctl_max_map_count __read_mostly = DEFAULT_MAX_MAP_COUNT;
struct percpu_counter vm_committed_as;

+/* amount of vm to protect from userspace access */
+unsigned long mmap_min_addr = CONFIG_DEFAULT_MMAP_MIN_ADDR;
+
/*
* Check that a process has enough memory to allocate a new virtual
* mapping. 0 means there is enough memory for the allocation to
@@ -1043,6 +1046,9 @@ unsigned long do_mmap_pgoff(struct file
}
}

+ if ((addr < mmap_min_addr) && !capable(CAP_SYS_RAWIO))
+ return -EACCES;
+
error = security_file_mmap(file, reqprot, prot, flags, addr, 0);
if (error)
return error;
Index: linux-2.6/security/security.c
===================================================================
--- linux-2.6.orig/security/security.c 2009-06-03 13:48:01.000000000 -0500
+++ linux-2.6/security/security.c 2009-06-03 13:48:10.000000000 -0500
@@ -26,9 +26,6 @@ extern void security_fixup_ops(struct se

struct security_operations *security_ops; /* Initialized to NULL */

-/* amount of vm to protect from userspace access */
-unsigned long mmap_min_addr = CONFIG_SECURITY_DEFAULT_MMAP_MIN_ADDR;
-
static inline int verify(struct security_operations *ops)
{
/* verify the security_operations structure exists */
Index: linux-2.6/mm/Kconfig
===================================================================
--- linux-2.6.orig/mm/Kconfig 2009-06-03 13:48:01.000000000 -0500
+++ linux-2.6/mm/Kconfig 2009-06-03 13:48:10.000000000 -0500
@@ -226,6 +226,25 @@ config HAVE_MLOCKED_PAGE_BIT
config MMU_NOTIFIER
bool

+config DEFAULT_MMAP_MIN_ADDR
+ int "Low address space to protect from user allocation"
+ default 4096
+ help
+ This is the portion of low virtual memory which should be protected
+ from userspace allocation. Keeping a user from writing to low pages
+ can help reduce the impact of kernel NULL pointer bugs.
+
+ For most ia64, ppc64 and x86 users with lots of address space
+ a value of 65536 is reasonable and should cause no problems.
+ On arm and other archs it should not be higher than 32768.
+ Programs which use vm86 functionality would either need additional
+ permissions from either the LSM or the capabilities module or have
+ this protection disabled.
+
+ This value can be changed after boot using the
+ /proc/sys/vm/mmap_min_addr tunable.
+
+
config NOMMU_INITIAL_TRIM_EXCESS
int "Turn on mmap() excess space trimming before booting"
depends on !MMU
Index: linux-2.6/security/Kconfig
===================================================================
--- linux-2.6.orig/security/Kconfig 2009-06-03 13:48:01.000000000 -0500
+++ linux-2.6/security/Kconfig 2009-06-03 13:48:10.000000000 -0500
@@ -113,26 +113,6 @@ config SECURITY_ROOTPLUG

If you are unsure how to answer this question, answer N.

-config SECURITY_DEFAULT_MMAP_MIN_ADDR
- int "Low address space to protect from user allocation"
- depends on SECURITY
- default 0
- help
- This is the portion of low virtual memory which should be protected
- from userspace allocation. Keeping a user from writing to low pages
- can help reduce the impact of kernel NULL pointer bugs.
-
- For most ia64, ppc64 and x86 users with lots of address space
- a value of 65536 is reasonable and should cause no problems.
- On arm and other archs it should not be higher than 32768.
- Programs which use vm86 functionality would either need additional
- permissions from either the LSM or the capabilities module or have
- this protection disabled.
-
- This value can be changed after boot using the
- /proc/sys/vm/mmap_min_addr tunable.
-
-
source security/selinux/Kconfig
source security/smack/Kconfig
source security/tomoyo/Kconfig
Index: linux-2.6/security/capability.c
===================================================================
--- linux-2.6.orig/security/capability.c 2009-06-03 13:48:01.000000000 -0500
+++ linux-2.6/security/capability.c 2009-06-03 13:48:10.000000000 -0500
@@ -334,8 +334,6 @@ static int cap_file_mmap(struct file *fi
unsigned long prot, unsigned long flags,
unsigned long addr, unsigned long addr_only)
{
- if ((addr < mmap_min_addr) && !capable(CAP_SYS_RAWIO))
- return -EACCES;
return 0;
}

2009-06-03 19:11:53

by Rik van Riel

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

Christoph Lameter wrote:
> We could just move the check for mmap_min_addr out from
> CONFIG_SECURITY?
>
>
> Use mmap_min_addr indepedently of security models
>
> This patch removes the dependency of mmap_min_addr on CONFIG_SECURITY.
> It also sets a default mmap_min_addr of 4096.
>
> mmapping of addresses below 4096 will only be possible for processes
> with CAP_SYS_RAWIO.
>
>
> Signed-off-by: Christoph Lameter <[email protected]>

Acked-by: Rik van Riel <[email protected]>

--
All rights reversed.

2009-06-03 19:14:58

by Eric Paris

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

On Wed, Jun 3, 2009 at 2:59 PM, Christoph Lameter
<[email protected]> wrote:
> We could just move the check for mmap_min_addr out from
> CONFIG_SECURITY?
>
>
> Use mmap_min_addr indepedently of security models
>
> This patch removes the dependency of mmap_min_addr on CONFIG_SECURITY.
> It also sets a default mmap_min_addr of 4096.
>
> mmapping of addresses below 4096 will only be possible for processes
> with CAP_SYS_RAWIO.
>
>
> Signed-off-by: Christoph Lameter <[email protected]>

NAK with SELinux on you now need both the SELinux mmap_zero
permission and the CAP_SYS_RAWIO permission. Previously you only
needed one or the other, depending on which was the predominant
LSM.....

Even if you want to argue that I have to take CAP_SYS_RAWIO in the
SELinux case what about all the other places? do_mremap? do_brk?
expand_downwards?

-Eric


> ===================================================================
> --- linux-2.6.orig/mm/mmap.c ? ?2009-06-03 13:48:01.000000000 -0500
> +++ linux-2.6/mm/mmap.c 2009-06-03 13:48:10.000000000 -0500
> @@ -87,6 +87,9 @@ int sysctl_overcommit_ratio = 50; ? ? /* def
> ?int sysctl_max_map_count __read_mostly = DEFAULT_MAX_MAP_COUNT;
> ?struct percpu_counter vm_committed_as;
>
> +/* amount of vm to protect from userspace access */
> +unsigned long mmap_min_addr = CONFIG_DEFAULT_MMAP_MIN_ADDR;
> +
> ?/*
> ?* Check that a process has enough memory to allocate a new virtual
> ?* mapping. 0 means there is enough memory for the allocation to
> @@ -1043,6 +1046,9 @@ unsigned long do_mmap_pgoff(struct file
> ? ? ? ? ? ? ? ?}
> ? ? ? ?}
>
> + ? ? ? if ((addr < mmap_min_addr) && !capable(CAP_SYS_RAWIO))
> + ? ? ? ? ? ? ? return -EACCES;
> +
> ? ? ? ?error = security_file_mmap(file, reqprot, prot, flags, addr, 0);
> ? ? ? ?if (error)
> ? ? ? ? ? ? ? ?return error;

2009-06-03 19:22:28

by Alan

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

On Wed, 3 Jun 2009 14:59:51 -0400 (EDT)
Christoph Lameter <[email protected]> wrote:

> We could just move the check for mmap_min_addr out from
> CONFIG_SECURITY?
>
>
> Use mmap_min_addr indepedently of security models
>
> This patch removes the dependency of mmap_min_addr on CONFIG_SECURITY.
> It also sets a default mmap_min_addr of 4096.
>
> mmapping of addresses below 4096 will only be possible for processes
> with CAP_SYS_RAWIO.

This appears to break the security models as they can no longer replace
the CAP_SYS_RAWIO check with something based on the security model.

> @@ -1043,6 +1046,9 @@ unsigned long do_mmap_pgoff(struct file
> }
> }
>
> + if ((addr < mmap_min_addr) && !capable(CAP_SYS_RAWIO))
> + return -EACCES;
> +

You can't move this bit here

> error = security_file_mmap(file, reqprot, prot, flags, addr, 0);

You need it in the default (no security) version of security_file_mmap()
in security.h not hard coded into do_mmap_pgoff, and leave the one in
cap_* alone.

So NAK - not to the idea but to the fact the patch is buggy.

Alan

2009-06-03 19:28:20

by Linus Torvalds

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)



On Wed, 3 Jun 2009, Christoph Lameter wrote:
>
> We could just move the check for mmap_min_addr out from
> CONFIG_SECURITY?

No.

The thing is, the security model wants to modify the rules on what's
"secure" and what isn't. And your patch just hard-coded that
capable(CAP_SYS_RAWIO) decision - but that's not what something like
SElinux actually uses to decide whether it's ok or not.

So if you do it in generic code, you'd have to make it much more complex.
One option would be to change the rule for what "security_file_mmap()"
means, and make the return value says "yes, no, override". Where
"override" would be "allow it for this process even if it's below the
minimum mmap limit.

But the better option really is to just copy the cap_file_mmap() rule to
the !SECURITY rule, and make !SECURITY really mean the same as "always do
default security", the way it's documented.

Linus

2009-06-03 19:42:33

by PaX Team

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

On 3 Jun 2009 at 11:45, Linus Torvalds wrote:

>
>
> On Wed, 3 Jun 2009, Larry H. wrote:
> > >
> > > The fact, the NULL pointer attack is neither easy nor common. It's
> > > perfectly reasonable to say "we'll allow mmap at virtual address zero".
> >
> > And how could you calibrate if this attack venue isn't easy to take
> > advantage of? Or not commonly abused? What empirical results led you to this
> > conclusion?
>
> It's not a primary attack vector. You need to have already broken local
> security to get there - you need to be able to execute code.

during last summer's flame war^W^Wdiscussion about how you guys were covering
up security fixes you brought an example of smart university students breaking
communal boxes left and right. are you now saying that it was actually a strawman
argument as you consider that situation already broken? you can't have it both
ways ;).

> That means that you've already by-passed all the main security. It's thus
> by definition less common than attack vectors like buffer overflows that
> give you that capability in the first place.

that only means that you've ignored multi-user boxes.

2009-06-03 19:42:44

by Christoph Lameter

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

On Wed, 3 Jun 2009, Eric Paris wrote:

> NAK with SELinux on you now need both the SELinux mmap_zero
> permission and the CAP_SYS_RAWIO permission. Previously you only
> needed one or the other, depending on which was the predominant
> LSM.....

CAP_SYS_RAWIO is checked so you only need to check for mmap_zero in
SELinux.

> Even if you want to argue that I have to take CAP_SYS_RAWIO in the
> SELinux case what about all the other places? do_mremap? do_brk?
> expand_downwards?

brk(0) would free up all the code? The others could be added.

2009-06-03 19:46:17

by Christoph Lameter

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

On Wed, 3 Jun 2009, Alan Cox wrote:

> This appears to break the security models as they can no longer replace
> the CAP_SYS_RAWIO check with something based on the security model.

Right it would be fixed like CAP_SYS_NICE.

>
> > @@ -1043,6 +1046,9 @@ unsigned long do_mmap_pgoff(struct file
> > }
> > }
> >
> > + if ((addr < mmap_min_addr) && !capable(CAP_SYS_RAWIO))
> > + return -EACCES;
> > +
>
> You can't move this bit here

The same code is executed in security_file_mmap right now which is the
next function called at this spot.

> > error = security_file_mmap(file, reqprot, prot, flags, addr, 0);
>
> You need it in the default (no security) version of security_file_mmap()
> in security.h not hard coded into do_mmap_pgoff, and leave the one in
> cap_* alone.

But that would still leave it up to the security "models" to check
for basic security issues.

2009-06-03 19:50:36

by Christoph Lameter

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

On Wed, 3 Jun 2009, Linus Torvalds wrote:

> But the better option really is to just copy the cap_file_mmap() rule to
> the !SECURITY rule, and make !SECURITY really mean the same as "always do
> default security", the way it's documented.

Na, I really like the ability to just avoid having to deal with this
"security" stuff (CONFIG_SECURITY). And core security checks sidelined in
some security model config thingy? I'd prefer to see these checks right
there in core code while working on them.

2009-06-03 19:51:22

by Eric Paris

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

On Wed, Jun 3, 2009 at 3:42 PM, Christoph Lameter
<[email protected]> wrote:
> On Wed, 3 Jun 2009, Eric Paris wrote:
>
>> NAK ?with SELinux on you now need both the SELinux mmap_zero
>> permission and the CAP_SYS_RAWIO permission. ?Previously you only
>> needed one or the other, depending on which was the predominant
>> LSM.....
>
> CAP_SYS_RAWIO is checked so you only need to check for mmap_zero in
> SELinux.

You misunderstand. As it stands today if you use SELinux you need
only the selinux mmap_zero permission. If you use capabilities you
need CAP_SYS_RAWIO.

With your patch SELinux policy would now have to grant CAP_SYS_RAWIO
everywhere it grants mmap_zero. This not not acceptable. Take notice
that with SELinux enabled cap_file_mmap is never called.....


>> Even if you want to argue that I have to take CAP_SYS_RAWIO in the
>> SELinux case what about all the other places? ?do_mremap? ?do_brk?
>> expand_downwards?
>
> brk(0) would free up all the code? The others could be added.

The 'right'est fix is as Alan suggested, duplicate the code

from security/capability.c::cap_file_mmap()
to include/linux/security.h::securitry_file_mmap()

-Eric

2009-06-03 20:01:45

by PaX Team

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

On 3 Jun 2009 at 11:50, Linus Torvalds wrote:

>
>
> On Wed, 3 Jun 2009, Linus Torvalds wrote:
> >
> > That means that you've already by-passed all the main security. It's thus
> > by definition less common than attack vectors like buffer overflows that
> > give you that capability in the first place.
>
> Btw, you obviously need to then _also_ pair it with some as-yet-unknown
> case of kernel bug to get to that NULL pointer (or zero-sized-alloc
> pointer) problem.

are you saying it's hard to find 'as-yet-unknown' null-deref bugs? what about
'already-known-but-not-yet-fixed-in-distro-kernel' ones? especially when the
disclosure process was, let's say, less than 'full'.

> You _also_ seem to be totally ignoring the fact that we already _do_
> protect against NULL pointers by default.

this whole discussion about NULL derefs is quite missing the point by the way.
the proper bug class is about unintended userland ptr derefs by the kernel,
of which NULL derefs are a small subset only. and you can't protect against
it by default or otherwise by banning userland from using its address space ;).

fixing ZERO_SIZE_PTR is about not making the mess of mixing userland/kernel
addresses worse, that's all. small piece of the parcel but then it's obviously
correct too.

> So I really don't see why you're making a big deal of this. It's as if you
> were talking about us not randomizing the address space - sure, you can
> turn it off, but so what? We do it by default.

and the amount of it is easily bruteforceable not to mention lack of
protection against said bruteforce. don't rest on your laurels yet ;).

> So it boils down to:
>
> - NULL pointers already cannot be in mmap memory

not all NULL deref bugs are literally around address 0, there's often an
offset involved, sometimes even under the attacker's control (a famous
userland example is discussed in
http://documents.iss.net/whitepapers/IBM_X-Force_WP_final.pdf
).

> (unless a distro has
> done something wrong - outside of the kernel)

do you have data about which distros and kernels enable this? and how
they handle v86 and stuff? suid root equivalent or something better?

2009-06-03 20:05:56

by Christoph Lameter

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

On Wed, 3 Jun 2009, Eric Paris wrote:

> The 'right'est fix is as Alan suggested, duplicate the code
>
> from security/capability.c::cap_file_mmap()
> to include/linux/security.h::securitry_file_mmap()

Thats easy to do but isnt it a bit weird now to configure mmap_min_addr?
A security model may give it a different interpretation?
What about round_hint_to_min()?


Use mmap_min_addr indepedently of security models

This patch removes the dependency of mmap_min_addr on CONFIG_SECURITY.
It also sets a default mmap_min_addr of 4096.

mmapping of addresses below 4096 will only be possible for processes
with CAP_SYS_RAWIO.


Signed-off-by: Christoph Lameter <[email protected]>

---
include/linux/mm.h | 2 --
include/linux/security.h | 2 ++
kernel/sysctl.c | 2 --
mm/Kconfig | 19 +++++++++++++++++++
mm/mmap.c | 3 +++
security/Kconfig | 20 --------------------
security/security.c | 3 ---
7 files changed, 24 insertions(+), 27 deletions(-)

Index: linux-2.6/include/linux/mm.h
===================================================================
--- linux-2.6.orig/include/linux/mm.h 2009-06-03 15:00:54.000000000 -0500
+++ linux-2.6/include/linux/mm.h 2009-06-03 15:00:56.000000000 -0500
@@ -580,12 +580,10 @@ static inline void set_page_links(struct
*/
static inline unsigned long round_hint_to_min(unsigned long hint)
{
-#ifdef CONFIG_SECURITY
hint &= PAGE_MASK;
if (((void *)hint != NULL) &&
(hint < mmap_min_addr))
return PAGE_ALIGN(mmap_min_addr);
-#endif
return hint;
}

Index: linux-2.6/kernel/sysctl.c
===================================================================
--- linux-2.6.orig/kernel/sysctl.c 2009-06-03 15:00:54.000000000 -0500
+++ linux-2.6/kernel/sysctl.c 2009-06-03 15:00:56.000000000 -0500
@@ -1225,7 +1225,6 @@ static struct ctl_table vm_table[] = {
.strategy = &sysctl_jiffies,
},
#endif
-#ifdef CONFIG_SECURITY
{
.ctl_name = CTL_UNNUMBERED,
.procname = "mmap_min_addr",
@@ -1234,7 +1233,6 @@ static struct ctl_table vm_table[] = {
.mode = 0644,
.proc_handler = &proc_doulongvec_minmax,
},
-#endif
#ifdef CONFIG_NUMA
{
.ctl_name = CTL_UNNUMBERED,
Index: linux-2.6/mm/mmap.c
===================================================================
--- linux-2.6.orig/mm/mmap.c 2009-06-03 15:00:54.000000000 -0500
+++ linux-2.6/mm/mmap.c 2009-06-03 15:01:18.000000000 -0500
@@ -87,6 +87,9 @@ int sysctl_overcommit_ratio = 50; /* def
int sysctl_max_map_count __read_mostly = DEFAULT_MAX_MAP_COUNT;
struct percpu_counter vm_committed_as;

+/* amount of vm to protect from userspace access */
+unsigned long mmap_min_addr = CONFIG_DEFAULT_MMAP_MIN_ADDR;
+
/*
* Check that a process has enough memory to allocate a new virtual
* mapping. 0 means there is enough memory for the allocation to
Index: linux-2.6/security/security.c
===================================================================
--- linux-2.6.orig/security/security.c 2009-06-03 15:00:54.000000000 -0500
+++ linux-2.6/security/security.c 2009-06-03 15:00:56.000000000 -0500
@@ -26,9 +26,6 @@ extern void security_fixup_ops(struct se

struct security_operations *security_ops; /* Initialized to NULL */

-/* amount of vm to protect from userspace access */
-unsigned long mmap_min_addr = CONFIG_SECURITY_DEFAULT_MMAP_MIN_ADDR;
-
static inline int verify(struct security_operations *ops)
{
/* verify the security_operations structure exists */
Index: linux-2.6/mm/Kconfig
===================================================================
--- linux-2.6.orig/mm/Kconfig 2009-06-03 15:00:54.000000000 -0500
+++ linux-2.6/mm/Kconfig 2009-06-03 15:00:56.000000000 -0500
@@ -226,6 +226,25 @@ config HAVE_MLOCKED_PAGE_BIT
config MMU_NOTIFIER
bool

+config DEFAULT_MMAP_MIN_ADDR
+ int "Low address space to protect from user allocation"
+ default 4096
+ help
+ This is the portion of low virtual memory which should be protected
+ from userspace allocation. Keeping a user from writing to low pages
+ can help reduce the impact of kernel NULL pointer bugs.
+
+ For most ia64, ppc64 and x86 users with lots of address space
+ a value of 65536 is reasonable and should cause no problems.
+ On arm and other archs it should not be higher than 32768.
+ Programs which use vm86 functionality would either need additional
+ permissions from either the LSM or the capabilities module or have
+ this protection disabled.
+
+ This value can be changed after boot using the
+ /proc/sys/vm/mmap_min_addr tunable.
+
+
config NOMMU_INITIAL_TRIM_EXCESS
int "Turn on mmap() excess space trimming before booting"
depends on !MMU
Index: linux-2.6/security/Kconfig
===================================================================
--- linux-2.6.orig/security/Kconfig 2009-06-03 15:00:54.000000000 -0500
+++ linux-2.6/security/Kconfig 2009-06-03 15:00:56.000000000 -0500
@@ -113,26 +113,6 @@ config SECURITY_ROOTPLUG

If you are unsure how to answer this question, answer N.

-config SECURITY_DEFAULT_MMAP_MIN_ADDR
- int "Low address space to protect from user allocation"
- depends on SECURITY
- default 0
- help
- This is the portion of low virtual memory which should be protected
- from userspace allocation. Keeping a user from writing to low pages
- can help reduce the impact of kernel NULL pointer bugs.
-
- For most ia64, ppc64 and x86 users with lots of address space
- a value of 65536 is reasonable and should cause no problems.
- On arm and other archs it should not be higher than 32768.
- Programs which use vm86 functionality would either need additional
- permissions from either the LSM or the capabilities module or have
- this protection disabled.
-
- This value can be changed after boot using the
- /proc/sys/vm/mmap_min_addr tunable.
-
-
source security/selinux/Kconfig
source security/smack/Kconfig
source security/tomoyo/Kconfig
Index: linux-2.6/include/linux/security.h
===================================================================
--- linux-2.6.orig/include/linux/security.h 2009-06-03 15:01:28.000000000 -0500
+++ linux-2.6/include/linux/security.h 2009-06-03 15:01:42.000000000 -0500
@@ -2197,6 +2197,8 @@ static inline int security_file_mmap(str
unsigned long addr,
unsigned long addr_only)
{
+ if ((addr < mmap_min_addr) && !capable(CAP_SYS_RAWIO))
+ return -EACCES;
return 0;
}

2009-06-03 20:16:16

by Eric Paris

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

On Wed, Jun 3, 2009 at 4:04 PM, Christoph Lameter
<[email protected]> wrote:
> On Wed, 3 Jun 2009, Eric Paris wrote:
>
>> The 'right'est fix is as Alan suggested, duplicate the code
>>
>> from security/capability.c::cap_file_mmap()
>> to include/linux/security.h::securitry_file_mmap()
>
> Thats easy to do but isnt it a bit weird now to configure mmap_min_addr?

??

> A security model may give it a different interpretation?

Not sure what you mean. Yes, each security model is allowed to decide
what permissions are needed to pass a given security check. SELinux
decided that CAP_SYS_RAWIO was not needed, but the selinux permission
mmap_zero was. Had there been a more specific capability to use
SELinux might have been happy using a capability...

> What about round_hint_to_min()?

not sure what you mean....

>
> Use mmap_min_addr indepedently of security models
>
> This patch removes the dependency of mmap_min_addr on CONFIG_SECURITY.
> It also sets a default mmap_min_addr of 4096.
>
> mmapping of addresses below 4096 will only be possible for processes
> with CAP_SYS_RAWIO.

<pedantic nit> "or the appropriate permission for the given LSM </pedantic nit>

> Signed-off-by: Christoph Lameter <[email protected]>

Clearly lots more cleanup can be done between CONFIG_SECURITY and
!CONFIG_SECURITY like Linus suggested, but

Acked-by: Eric Paris <[email protected]>

> ---
> ?include/linux/mm.h ? ? ? | ? ?2 --
> ?include/linux/security.h | ? ?2 ++
> ?kernel/sysctl.c ? ? ? ? ?| ? ?2 --
> ?mm/Kconfig ? ? ? ? ? ? ? | ? 19 +++++++++++++++++++
> ?mm/mmap.c ? ? ? ? ? ? ? ?| ? ?3 +++
> ?security/Kconfig ? ? ? ? | ? 20 --------------------
> ?security/security.c ? ? ?| ? ?3 ---
> ?7 files changed, 24 insertions(+), 27 deletions(-)
>
> Index: linux-2.6/include/linux/mm.h
> ===================================================================
> --- linux-2.6.orig/include/linux/mm.h ? 2009-06-03 15:00:54.000000000 -0500
> +++ linux-2.6/include/linux/mm.h ? ? ? ?2009-06-03 15:00:56.000000000 -0500
> @@ -580,12 +580,10 @@ static inline void set_page_links(struct
> ?*/
> ?static inline unsigned long round_hint_to_min(unsigned long hint)
> ?{
> -#ifdef CONFIG_SECURITY
> ? ? ? ?hint &= PAGE_MASK;
> ? ? ? ?if (((void *)hint != NULL) &&
> ? ? ? ? ? ?(hint < mmap_min_addr))
> ? ? ? ? ? ? ? ?return PAGE_ALIGN(mmap_min_addr);
> -#endif
> ? ? ? ?return hint;
> ?}
>
> Index: linux-2.6/kernel/sysctl.c
> ===================================================================
> --- linux-2.6.orig/kernel/sysctl.c ? ? ?2009-06-03 15:00:54.000000000 -0500
> +++ linux-2.6/kernel/sysctl.c ? 2009-06-03 15:00:56.000000000 -0500
> @@ -1225,7 +1225,6 @@ static struct ctl_table vm_table[] = {
> ? ? ? ? ? ? ? ?.strategy ? ? ? = &sysctl_jiffies,
> ? ? ? ?},
> ?#endif
> -#ifdef CONFIG_SECURITY
> ? ? ? ?{
> ? ? ? ? ? ? ? ?.ctl_name ? ? ? = CTL_UNNUMBERED,
> ? ? ? ? ? ? ? ?.procname ? ? ? = "mmap_min_addr",
> @@ -1234,7 +1233,6 @@ static struct ctl_table vm_table[] = {
> ? ? ? ? ? ? ? ?.mode ? ? ? ? ? = 0644,
> ? ? ? ? ? ? ? ?.proc_handler ? = &proc_doulongvec_minmax,
> ? ? ? ?},
> -#endif
> ?#ifdef CONFIG_NUMA
> ? ? ? ?{
> ? ? ? ? ? ? ? ?.ctl_name ? ? ? = CTL_UNNUMBERED,
> Index: linux-2.6/mm/mmap.c
> ===================================================================
> --- linux-2.6.orig/mm/mmap.c ? ?2009-06-03 15:00:54.000000000 -0500
> +++ linux-2.6/mm/mmap.c 2009-06-03 15:01:18.000000000 -0500
> @@ -87,6 +87,9 @@ int sysctl_overcommit_ratio = 50; ? ? /* def
> ?int sysctl_max_map_count __read_mostly = DEFAULT_MAX_MAP_COUNT;
> ?struct percpu_counter vm_committed_as;
>
> +/* amount of vm to protect from userspace access */
> +unsigned long mmap_min_addr = CONFIG_DEFAULT_MMAP_MIN_ADDR;
> +
> ?/*
> ?* Check that a process has enough memory to allocate a new virtual
> ?* mapping. 0 means there is enough memory for the allocation to
> Index: linux-2.6/security/security.c
> ===================================================================
> --- linux-2.6.orig/security/security.c ?2009-06-03 15:00:54.000000000 -0500
> +++ linux-2.6/security/security.c ? ? ? 2009-06-03 15:00:56.000000000 -0500
> @@ -26,9 +26,6 @@ extern void security_fixup_ops(struct se
>
> ?struct security_operations *security_ops; ? ? ?/* Initialized to NULL */
>
> -/* amount of vm to protect from userspace access */
> -unsigned long mmap_min_addr = CONFIG_SECURITY_DEFAULT_MMAP_MIN_ADDR;
> -
> ?static inline int verify(struct security_operations *ops)
> ?{
> ? ? ? ?/* verify the security_operations structure exists */
> Index: linux-2.6/mm/Kconfig
> ===================================================================
> --- linux-2.6.orig/mm/Kconfig ? 2009-06-03 15:00:54.000000000 -0500
> +++ linux-2.6/mm/Kconfig ? ? ? ?2009-06-03 15:00:56.000000000 -0500
> @@ -226,6 +226,25 @@ config HAVE_MLOCKED_PAGE_BIT
> ?config MMU_NOTIFIER
> ? ? ? ?bool
>
> +config DEFAULT_MMAP_MIN_ADDR
> + ? ? ? ?int "Low address space to protect from user allocation"
> + ? ? ? ?default 4096
> + ? ? ? ?help
> + ? ? ? ? This is the portion of low virtual memory which should be protected
> + ? ? ? ? from userspace allocation. ?Keeping a user from writing to low pages
> + ? ? ? ? can help reduce the impact of kernel NULL pointer bugs.
> +
> + ? ? ? ? For most ia64, ppc64 and x86 users with lots of address space
> + ? ? ? ? a value of 65536 is reasonable and should cause no problems.
> + ? ? ? ? On arm and other archs it should not be higher than 32768.
> + ? ? ? ? Programs which use vm86 functionality would either need additional
> + ? ? ? ? permissions from either the LSM or the capabilities module or have
> + ? ? ? ? this protection disabled.
> +
> + ? ? ? ? This value can be changed after boot using the
> + ? ? ? ? /proc/sys/vm/mmap_min_addr tunable.
> +
> +
> ?config NOMMU_INITIAL_TRIM_EXCESS
> ? ? ? ?int "Turn on mmap() excess space trimming before booting"
> ? ? ? ?depends on !MMU
> Index: linux-2.6/security/Kconfig
> ===================================================================
> --- linux-2.6.orig/security/Kconfig ? ? 2009-06-03 15:00:54.000000000 -0500
> +++ linux-2.6/security/Kconfig ?2009-06-03 15:00:56.000000000 -0500
> @@ -113,26 +113,6 @@ config SECURITY_ROOTPLUG
>
> ? ? ? ? ?If you are unsure how to answer this question, answer N.
>
> -config SECURITY_DEFAULT_MMAP_MIN_ADDR
> - ? ? ? ?int "Low address space to protect from user allocation"
> - ? ? ? ?depends on SECURITY
> - ? ? ? ?default 0
> - ? ? ? ?help
> - ? ? ? ? This is the portion of low virtual memory which should be protected
> - ? ? ? ? from userspace allocation. ?Keeping a user from writing to low pages
> - ? ? ? ? can help reduce the impact of kernel NULL pointer bugs.
> -
> - ? ? ? ? For most ia64, ppc64 and x86 users with lots of address space
> - ? ? ? ? a value of 65536 is reasonable and should cause no problems.
> - ? ? ? ? On arm and other archs it should not be higher than 32768.
> - ? ? ? ? Programs which use vm86 functionality would either need additional
> - ? ? ? ? permissions from either the LSM or the capabilities module or have
> - ? ? ? ? this protection disabled.
> -
> - ? ? ? ? This value can be changed after boot using the
> - ? ? ? ? /proc/sys/vm/mmap_min_addr tunable.
> -
> -
> ?source security/selinux/Kconfig
> ?source security/smack/Kconfig
> ?source security/tomoyo/Kconfig
> Index: linux-2.6/include/linux/security.h
> ===================================================================
> --- linux-2.6.orig/include/linux/security.h ? ? 2009-06-03 15:01:28.000000000 -0500
> +++ linux-2.6/include/linux/security.h ?2009-06-03 15:01:42.000000000 -0500
> @@ -2197,6 +2197,8 @@ static inline int security_file_mmap(str
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned long addr,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned long addr_only)
> ?{
> + ? ? ? if ((addr < mmap_min_addr) && !capable(CAP_SYS_RAWIO))
> + ? ? ? ? ? ? ? return -EACCES;
> ? ? ? ?return 0;
> ?}
>
>

2009-06-03 20:36:42

by Christoph Lameter

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

On Wed, 3 Jun 2009, Eric Paris wrote:

> > Thats easy to do but isnt it a bit weird now to configure mmap_min_addr?
>
> ??

The use of mmap_min_addr depends on the security configuration chose. The
security model may not check at all. But we can still configure the thing.

> > What about round_hint_to_min()?
>
> not sure what you mean....

We removed the CONFIG_SECURITY around code in there in the patch.

2009-06-03 21:07:28

by Alan

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

> > You need it in the default (no security) version of security_file_mmap()
> > in security.h not hard coded into do_mmap_pgoff, and leave the one in
> > cap_* alone.
>
> But that would still leave it up to the security "models" to check
> for basic security issues.

Correct. You have no knowledge of the policy at the higher level. In the
SELinux case security labels are used to identify code which is permitted
to map low pages. That means the root/RAW_IO security sledgehammer can be
replaced with a more secure labelling system.

Other policy systems might do it on namespaces (perhaps /bin
and /usr/bin mapping zero OK, /home not etc)

2009-06-03 21:21:54

by Linus Torvalds

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)



On Wed, 3 Jun 2009, Christoph Lameter wrote:
>
> Use mmap_min_addr indepedently of security models

Looks ok by me. As mentioned, it would be nice if the coherency with the
'capabilities' security module was something inherent to the code, but
this looks like a sane minimal patch.

Linus

2009-06-03 22:53:21

by James Morris

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

On Wed, 3 Jun 2009, Larry H. wrote:

> whenever it is feasible, IMHO. I think everyone here will agree that
> SELinux has a track of being disabled by users after installation
> because they don't want to invest the necessary time on understanding
> and learning the policy language or management tools.

The Fedora smolt stats show an overwhelming majority of people leave it
running. Many don't know it's there at all and never have problems.
It's known to have saved many everyday systems from breaches.

That's not to say that a significant number of people don't disable it,
similarly to the way people disable iptables, use weak passwords, drive
without seat belts, and cycle without helmets. We do need to try and keep
the default as safe as possible.


- James
--
James Morris
<[email protected]>

2009-06-04 02:42:25

by James Morris

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

On Wed, 3 Jun 2009, Christoph Lameter wrote:

> Use mmap_min_addr indepedently of security models
>
> This patch removes the dependency of mmap_min_addr on CONFIG_SECURITY.
> It also sets a default mmap_min_addr of 4096.
>
> mmapping of addresses below 4096 will only be possible for processes
> with CAP_SYS_RAWIO.
>
>
> Signed-off-by: Christoph Lameter <[email protected]>


Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next

>
> ---
> include/linux/mm.h | 2 --
> include/linux/security.h | 2 ++
> kernel/sysctl.c | 2 --
> mm/Kconfig | 19 +++++++++++++++++++
> mm/mmap.c | 3 +++
> security/Kconfig | 20 --------------------
> security/security.c | 3 ---
> 7 files changed, 24 insertions(+), 27 deletions(-)
>
> Index: linux-2.6/include/linux/mm.h
> ===================================================================
> --- linux-2.6.orig/include/linux/mm.h 2009-06-03 15:00:54.000000000 -0500
> +++ linux-2.6/include/linux/mm.h 2009-06-03 15:00:56.000000000 -0500
> @@ -580,12 +580,10 @@ static inline void set_page_links(struct
> */
> static inline unsigned long round_hint_to_min(unsigned long hint)
> {
> -#ifdef CONFIG_SECURITY
> hint &= PAGE_MASK;
> if (((void *)hint != NULL) &&
> (hint < mmap_min_addr))
> return PAGE_ALIGN(mmap_min_addr);
> -#endif
> return hint;
> }
>
> Index: linux-2.6/kernel/sysctl.c
> ===================================================================
> --- linux-2.6.orig/kernel/sysctl.c 2009-06-03 15:00:54.000000000 -0500
> +++ linux-2.6/kernel/sysctl.c 2009-06-03 15:00:56.000000000 -0500
> @@ -1225,7 +1225,6 @@ static struct ctl_table vm_table[] = {
> .strategy = &sysctl_jiffies,
> },
> #endif
> -#ifdef CONFIG_SECURITY
> {
> .ctl_name = CTL_UNNUMBERED,
> .procname = "mmap_min_addr",
> @@ -1234,7 +1233,6 @@ static struct ctl_table vm_table[] = {
> .mode = 0644,
> .proc_handler = &proc_doulongvec_minmax,
> },
> -#endif
> #ifdef CONFIG_NUMA
> {
> .ctl_name = CTL_UNNUMBERED,
> Index: linux-2.6/mm/mmap.c
> ===================================================================
> --- linux-2.6.orig/mm/mmap.c 2009-06-03 15:00:54.000000000 -0500
> +++ linux-2.6/mm/mmap.c 2009-06-03 15:01:18.000000000 -0500
> @@ -87,6 +87,9 @@ int sysctl_overcommit_ratio = 50; /* def
> int sysctl_max_map_count __read_mostly = DEFAULT_MAX_MAP_COUNT;
> struct percpu_counter vm_committed_as;
>
> +/* amount of vm to protect from userspace access */
> +unsigned long mmap_min_addr = CONFIG_DEFAULT_MMAP_MIN_ADDR;
> +
> /*
> * Check that a process has enough memory to allocate a new virtual
> * mapping. 0 means there is enough memory for the allocation to
> Index: linux-2.6/security/security.c
> ===================================================================
> --- linux-2.6.orig/security/security.c 2009-06-03 15:00:54.000000000 -0500
> +++ linux-2.6/security/security.c 2009-06-03 15:00:56.000000000 -0500
> @@ -26,9 +26,6 @@ extern void security_fixup_ops(struct se
>
> struct security_operations *security_ops; /* Initialized to NULL */
>
> -/* amount of vm to protect from userspace access */
> -unsigned long mmap_min_addr = CONFIG_SECURITY_DEFAULT_MMAP_MIN_ADDR;
> -
> static inline int verify(struct security_operations *ops)
> {
> /* verify the security_operations structure exists */
> Index: linux-2.6/mm/Kconfig
> ===================================================================
> --- linux-2.6.orig/mm/Kconfig 2009-06-03 15:00:54.000000000 -0500
> +++ linux-2.6/mm/Kconfig 2009-06-03 15:00:56.000000000 -0500
> @@ -226,6 +226,25 @@ config HAVE_MLOCKED_PAGE_BIT
> config MMU_NOTIFIER
> bool
>
> +config DEFAULT_MMAP_MIN_ADDR
> + int "Low address space to protect from user allocation"
> + default 4096
> + help
> + This is the portion of low virtual memory which should be protected
> + from userspace allocation. Keeping a user from writing to low pages
> + can help reduce the impact of kernel NULL pointer bugs.
> +
> + For most ia64, ppc64 and x86 users with lots of address space
> + a value of 65536 is reasonable and should cause no problems.
> + On arm and other archs it should not be higher than 32768.
> + Programs which use vm86 functionality would either need additional
> + permissions from either the LSM or the capabilities module or have
> + this protection disabled.
> +
> + This value can be changed after boot using the
> + /proc/sys/vm/mmap_min_addr tunable.
> +
> +
> config NOMMU_INITIAL_TRIM_EXCESS
> int "Turn on mmap() excess space trimming before booting"
> depends on !MMU
> Index: linux-2.6/security/Kconfig
> ===================================================================
> --- linux-2.6.orig/security/Kconfig 2009-06-03 15:00:54.000000000 -0500
> +++ linux-2.6/security/Kconfig 2009-06-03 15:00:56.000000000 -0500
> @@ -113,26 +113,6 @@ config SECURITY_ROOTPLUG
>
> If you are unsure how to answer this question, answer N.
>
> -config SECURITY_DEFAULT_MMAP_MIN_ADDR
> - int "Low address space to protect from user allocation"
> - depends on SECURITY
> - default 0
> - help
> - This is the portion of low virtual memory which should be protected
> - from userspace allocation. Keeping a user from writing to low pages
> - can help reduce the impact of kernel NULL pointer bugs.
> -
> - For most ia64, ppc64 and x86 users with lots of address space
> - a value of 65536 is reasonable and should cause no problems.
> - On arm and other archs it should not be higher than 32768.
> - Programs which use vm86 functionality would either need additional
> - permissions from either the LSM or the capabilities module or have
> - this protection disabled.
> -
> - This value can be changed after boot using the
> - /proc/sys/vm/mmap_min_addr tunable.
> -
> -
> source security/selinux/Kconfig
> source security/smack/Kconfig
> source security/tomoyo/Kconfig
> Index: linux-2.6/include/linux/security.h
> ===================================================================
> --- linux-2.6.orig/include/linux/security.h 2009-06-03 15:01:28.000000000 -0500
> +++ linux-2.6/include/linux/security.h 2009-06-03 15:01:42.000000000 -0500
> @@ -2197,6 +2197,8 @@ static inline int security_file_mmap(str
> unsigned long addr,
> unsigned long addr_only)
> {
> + if ((addr < mmap_min_addr) && !capable(CAP_SYS_RAWIO))
> + return -EACCES;
> return 0;
> }
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to [email protected]. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
>

--
James Morris
<[email protected]>

2009-06-07 10:29:54

by Pavel Machek

[permalink] [raw]
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)

Hi!

> Ok. So what we need to do is stop this toying around with remapping of
> page 0. The following patch contains a fix and a test program that
> demonstrates the issue.
>
>
> Subject: [Security] Do not allow remapping of page 0 via MAP_FIXED
>
> If one remaps page 0 then the kernel checks for NULL pointers of various
> flavors are bypassed and this may be exploited in various creative ways
> to transfer data from kernel space to user space.

Yes, mmap() at page zero 0 makes exploits harder; and yes disabling it
may be useful (but we tried that before, see Alan's comment). But that
does not it mean it deserves _security_ label. Call it robustness or
something....
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html