Fix SELinux warnings when running on Fedora....

Fix SELinux warnings when running on Fedora.
(c.f. http://people.redhat.com/drepper/selinux-mem.html)

Fix compilation warnings on Fedora.

BUG=none
TEST=when running Chrome on Fedora, verify that we don't get AVC warnings

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=43107

[Markus (顧孟勤), 2010-03-30 16:10:59]

While patching system calls, we would previously change privileges from
PROT_READ|PROT_EXEC to PROT_READ|PROT_WRITE, and after making our changes, we
would then change back to PROT_READ|PROT_EXEC.

The second transition raises warning messages on Fedora. It doesn't like adding
PROT_EXEC permissions to a mapping that was mapped PROT_WRITE. But if we never
drop PROT_EXEC in the first place, we can make the warning messages go away.

As there isn't really any security improvement in temporarily dropping
PROT_EXEC, I decided to keep it active the entire time.

I built a copy of 32bit Chrome in Fedora 12 (which takes forever in QEmu and
over sshfs :-). I verified that it no longer displays error messages when
running Chrome in the seccomp sandbox.

[Evan Martin, 2010-03-30 16:11:33]

=> selinux master before i even look at it

[Evan Martin, 2010-03-30 16:13:24]

I can't review this intelligently.  I thought RWX pages were generally bad,
though.

http://codereview.chromium.org/1535004/diff/1/3
File sandbox/linux/seccomp/library.h (right):

http://codereview.chromium.org/1535004/diff/1/3#newcode129
sandbox/linux/seccomp/library.h:129: int  getSectionIndex(const string&
section);
probably only meant one space here

[agl, 2010-03-30 16:14:11]

LGTM.

This is controlled by a policy boolean which we probably need to flip for V8
anyway. I'm not sure why I don't see it for V8: possibly because I've configured
my systems for Go which already does runtime code gen.

(Also, you can always ask me to test these things. My Z600 runs FC12.)

[agl, 2010-03-30 16:16:03]

On Tue, Mar 30, 2010 at 12:13 PM,  <evan@chromium.org> wrote:
> I can't review this intelligently.  I thought RWX pages were generally bad,
> though.

Yea, it does beg the question as to why we don't go RX -> RWX -> RX?

AGL

[Markus (顧孟勤), 2010-03-30 16:25:41]

On Tue, Mar 30, 2010 at 09:15, Adam Langley <agl@chromium.org> wrote:

> On Tue, Mar 30, 2010 at 12:13 PM,  <evan@chromium.org> wrote:
> > I can't review this intelligently.  I thought RWX pages were generally
> bad,
> > though.
>

They are bad. But only really from a philosophical standpoint. Any time you
have an RWX page, an attacker can write arbitrary code into this page. So,
during normal program execution, you don't want any of these pages around.
But if you do code generation (jit compiler, runtime patching, ...) then
that's exactly what you want to do. You just generally try to change the
pages back to RX as soon as possible. And you also try to avoid having them
RWX at a time when attackers can launch their attacks. That's why we enable
the sandbox before we start the actual renderers.


> Yea, it does beg the question as to why we don't go RX -> RWX -> RX?
>

That's what this patch does, and it appears to make SELinux happy. From a
purely philosophical point of view, I liked RX - > RW -> RX better. But I
don't think it really makes a difference.

I suspect, V8 does the same thing, because I have never seen it throwing up
any of the AVC warnings. And Go should probably do so, too, if you need to
generate code on the fly.


Markus

[agl, 2010-03-30 16:28:41]

On Tue, Mar 30, 2010 at 12:25 PM, Markus Gutschke <markus@chromium.org> wrote:
>> Yea, it does beg the question as to why we don't go RX -> RWX -> RX?
>
> That's what this patch does, and it appears to make SELinux happy. From a

Ah, ok. I guess the diff doesn't include the RWX -> RX change. It
looked like everything was going to RWX.


LGTM


AGL