code review 5650072: runtime: fix grsec support

runtime: fix grsec support

Changeset 36c9c7810f14 broke support for grsec-patched kernels.
Those do not give back the address requested without MAP_FIXED,
so when verifying an mmap without this flag for success, the
resulting address must not be compared against the requested
address since it may have succeeded at a different location.

[niemeyer, 2012-02-13 03:02:38]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/

[rsc, 2012-02-13 03:43:32]

Not just add MAP_FIXED?

[gustavo_niemeyer.net, 2012-02-13 03:48:32]

On Mon, Feb 13, 2012 at 01:43, Russ Cox <rsc@golang.org> wrote:
> Not just add MAP_FIXED?

Should work too, but we don't actually care about the address, and the
same logic was used a few lines below.

Whatever you prefer, though.

-- 
Gustavo Niemeyer
http://niemeyer.net
http://niemeyer.net/plus
http://niemeyer.net/twitter
http://niemeyer.net/blog

-- I'm not absolutely sure of anything.

[gustavo_niemeyer.net, 2012-02-13 04:11:51]

On Mon, Feb 13, 2012 at 01:48, Gustavo Niemeyer <gustavo@niemeyer.net> wrote:
> On Mon, Feb 13, 2012 at 01:43, Russ Cox <rsc@golang.org> wrote:
>> Not just add MAP_FIXED?
>
> Should work too, but we don't actually care about the address, and the
> same logic was used a few lines below.
>
> Whatever you prefer, though.

Actually, if we do use a MAP_FIXED here, wouldn't it be a bit
pointless to have the extra care in SysMap to prevent stealing a
memory page unintentionally?

-- 
Gustavo Niemeyer
http://niemeyer.net
http://niemeyer.net/plus
http://niemeyer.net/twitter
http://niemeyer.net/blog

-- I'm not absolutely sure of anything.

[rsc, 2012-02-13 04:30:18]

LGTM

Let's try without MAP_FIXED and see if it breaks UML.

[iant2, 2012-02-13 05:59:47]

FYI

Russ Cox <rsc@golang.org> writes:

> Not just add MAP_FIXED?

It's generally unwise to use MAP_FIXED without first calling something
like addrspace_free, because MAP_FIXED will override any existing
mapping.

Ian

[gustavo_niemeyer.net, 2012-02-13 15:06:59]

On Mon, Feb 13, 2012 at 02:30, Russ Cox <rsc@golang.org> wrote:
> Let's try without MAP_FIXED and see if it breaks UML.

It actually breaks it, sorry. But it's trivial to do a better job and fix both.

PTAL

-- 
Gustavo Niemeyer
http://niemeyer.net
http://niemeyer.net/plus
http://niemeyer.net/twitter
http://niemeyer.net/blog

-- I'm not absolutely sure of anything.

[rsc, 2012-02-14 16:18:59]

LGTM

http://codereview.appspot.com/5650072/diff/2003/src/pkg/runtime/mem_linux.c
File src/pkg/runtime/mem_linux.c (right):

http://codereview.appspot.com/5650072/diff/2003/src/pkg/runtime/mem_linux.c#n...
src/pkg/runtime/mem_linux.c:38: mmap_fixed(byte *v, uintptr n, int32 prot, int32
flags, int32 fd, uint32 offset) {
brace on next line please

[niemeyer, 2012-02-14 16:21:27]

http://codereview.appspot.com/5650072/diff/2003/src/pkg/runtime/mem_linux.c
File src/pkg/runtime/mem_linux.c (right):

http://codereview.appspot.com/5650072/diff/2003/src/pkg/runtime/mem_linux.c#n...
src/pkg/runtime/mem_linux.c:38: mmap_fixed(byte *v, uintptr n, int32 prot, int32
flags, int32 fd, uint32 offset) {
On 2012/02/14 16:18:59, rsc wrote:
> brace on next line please

Ouch.. I can't write C anymore.

[niemeyer, 2012-02-15 00:09:09]

*** Submitted as http://code.google.com/p/go/source/detail?r=9bd6434d05fa ***

runtime: fix grsec support

Changeset 36c9c7810f14 broke support for grsec-patched kernels.
Those do not give back the address requested without MAP_FIXED,
so when verifying an mmap without this flag for success, the
resulting address must not be compared against the requested
address since it may have succeeded at a different location.

R=golang-dev, rsc, gustavo, iant
CC=golang-dev
http://codereview.appspot.com/5650072