linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: "Larry H." <research@subreption.com>
To: Christoph Lameter <cl@linux-foundation.org>
Cc: Stephen Smalley <sds@tycho.nsa.gov>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	linux-mm@kvack.org, Alan Cox <alan@lxorguk.ukuu.org.uk>,
	Rik van Riel <riel@redhat.com>,
	linux-kernel@vger.kernel.org, pageexec@freemail.hu
Subject: Re: Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space)
Date: Wed, 3 Jun 2009 09:28:31 -0700	[thread overview]
Message-ID: <20090603162831.GF6701@oblivion.subreption.com> (raw)
In-Reply-To: <alpine.DEB.1.10.0906031134410.13551@gentwo.org>

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

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  parent reply	other threads:[~2009-06-03 16:26 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-30 19:28 [PATCH] Change ZERO_SIZE_PTR to point at unmapped space Larry H.
2009-05-30 22:29 ` Linus Torvalds
2009-05-30 23:00   ` Larry H.
2009-05-31  2:02     ` Linus Torvalds
2009-05-31  2:21       ` Larry H.
2009-06-02 15:37         ` Christoph Lameter
2009-06-02 20:34           ` Larry H.
2009-06-03 14:50             ` Security fix for remapping of page 0 (was [PATCH] Change ZERO_SIZE_PTR to point at unmapped space) Christoph Lameter
2009-06-03 15:07               ` Linus Torvalds
2009-06-03 15:23                 ` Christoph Lameter
2009-06-03 15:38                   ` Linus Torvalds
2009-06-03 16:14                     ` Alan Cox
2009-06-03 16:19                       ` Linus Torvalds
2009-06-03 16:24                         ` Eric Paris
2009-06-03 16:22                     ` Eric Paris
2009-06-03 16:28                       ` Linus Torvalds
2009-06-03 16:32                         ` Eric Paris
2009-06-03 16:44                           ` Linus Torvalds
2009-06-03 15:11               ` Stephen Smalley
2009-06-03 15:41                 ` Christoph Lameter
2009-06-03 16:18                   ` Linus Torvalds
2009-06-03 16:28                   ` Larry H. [this message]
2009-06-03 16:36                     ` Rik van Riel
2009-06-03 16:47                       ` Linus Torvalds
2009-06-03 17:16                         ` Eric Paris
2009-06-03 17:28                           ` Linus Torvalds
2009-06-03 17:31                             ` Eric Paris
2009-06-03 17:24                         ` Larry H.
2009-06-03 17:21                       ` Larry H.
2009-06-03 22:52                         ` James Morris
2009-06-03 17:29               ` Alan Cox
2009-06-03 17:35                 ` Linus Torvalds
2009-06-03 18:00                   ` Larry H.
2009-06-03 18:12                     ` Linus Torvalds
2009-06-03 18:39                       ` Larry H.
2009-06-03 18:45                         ` Linus Torvalds
2009-06-03 18:50                           ` Linus Torvalds
2009-06-03 18:59                             ` Christoph Lameter
2009-06-03 19:11                               ` Rik van Riel
2009-06-03 19:14                               ` Eric Paris
2009-06-03 19:42                                 ` Christoph Lameter
2009-06-03 19:51                                   ` Eric Paris
2009-06-03 20:04                                     ` Christoph Lameter
2009-06-03 20:16                                       ` Eric Paris
2009-06-03 20:36                                         ` Christoph Lameter
2009-06-03 21:20                                       ` Linus Torvalds
2009-06-04  2:41                                       ` James Morris
2009-06-03 19:21                               ` Alan Cox
2009-06-03 19:45                                 ` Christoph Lameter
2009-06-03 21:07                                   ` Alan Cox
2009-06-03 19:27                               ` Linus Torvalds
2009-06-03 19:50                                 ` Christoph Lameter
2009-06-03 20:00                             ` pageexec
2009-06-03 19:41                           ` pageexec
2009-06-07 10:29               ` Pavel Machek
2009-05-30 22:32 ` [PATCH] Change ZERO_SIZE_PTR to point at unmapped space Peter Zijlstra
2009-05-30 22:51   ` Larry H.

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090603162831.GF6701@oblivion.subreption.com \
    --to=research@subreption.com \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=cl@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=pageexec@freemail.hu \
    --cc=riel@redhat.com \
    --cc=sds@tycho.nsa.gov \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox