Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 05 Feb 1999 14:05:11 -0800 (PST)
From:      John Polstra <jdp@polstra.com>
To:        Matthew Dillon <dillon@apollo.backplane.com>
Cc:        committers@FreeBSD.ORG
Subject:   Re: cvs commit: src/sys/vm vm_unix.c
Message-ID:  <XFMail.990205140511.jdp@polstra.com>
In-Reply-To: <199902052105.NAA99296@apollo.backplane.com>

next in thread | previous in thread | raw e-mail | index | archive | help
Matthew Dillon wrote:
[jdp:]
>:Wouldn't this change break things like just-in-time compilers, if
>:it weren't for the quirk that our currently-supported architectures
>:ignore VM_PROT_EXECUTE?
>:
>:I think it would be more correct to take the opposite approach and
>:make the ELF loader use VM_PROT_ALL.  That's what is done for a.out.
>:I believe the stack already needs to be VM_PROT_ALL, because of the
>:signal trampoline code.
>:
>:I think we should change the ELF loader and RTLD to grant execute
>:permission whenever read permission is present.  I'm willing to do
>:the work if there's agreement.

>     It shouldn't have an effect, simply because IA32 does not have
>     an execute flag in its pte.

But this is architecture-independent code, so it must not rely on
that.  On some architectures, execute permission really is distinct
from read permission.  Obviously on architectures where execute
permission is ignored, your change is a no-op and we have nothing to
discuss.  Therefore in the following I am going to speak to the case
where execute permission matters.

And in that case, the data segment must be VM_PROT_ALL for a very
important reason that I hadn't thought of originally -- namely, lazy
binding of function calls in dynamically linked programs.  This is
implemented by executing code that is in the GOT (global offset
table), which is in the data segment because it must be writable.
Since code is executed there, it must be executable as well.

>     Also, dynamic loaders and ( I expect ) JIT compilers use mmap()
>     to allocate space.

I checked the kaffe sources, and by default it doesn't use mmap().
You can force it to do so with a configure option, but our FreeBSD
port doesn't do that currently.  Yet JIT works in kaffe under FreeBSD.

>     Using malloc() is rather dangerous since you can't be sure that
>     the memory is pristine from the point of view of the instruction
>     cache.

Kaffe must have found some way to solve that problem.

>     Plus, a JIT compiler would also use mprotect().

Kaffe doesn't use mprotect().

>     So, given all of that plus the fact that the 'default' should
>     be 'more secure' rather then 'less secure', I think it makes
>     more sense to use VM_PROT_READ/WRITE rather then VM_PROT_ALL.

I don't think security enters into this particular change, though.
The only place where removing VM_PROT_EXECUTE would be likely to aid
security would be in the stack segment, to protect against buffer
overflow attacks.  But the stack segment is the one place where we
can't turn off VM_PROT_EXECUTE, because of the sigtramp code there.

>From a security standpoint, being able to execute code in the data
segment doesn't make much difference unless an attacker has a way to
jump to it.  I won't claim it's 100% impossible in all programs to do
that without using stack buffer overflows, but it's at least a whole
lot harder.  And if attacker can cause a stack buffer overflow, he
might as well put the code he wants to execute on the stack too.

For the above reasons, I still think this change is the wrong way to
go.

John
---
  John Polstra                                               jdp@polstra.com
  John D. Polstra & Co., Inc.                        Seattle, Washington USA
  "Nobody ever went broke underestimating the taste of the American public."
                                                            -- H. L. Mencken

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?XFMail.990205140511.jdp>