Executing from readablee, no-exec pages

As revealed by the recent "Prevent data exception in kernel space" 
patch, versions of glibc prior to 2.4[1] assume that, on powerpc32, they 
can execute out of any readable mapping, regardless of whether it is 
marked for execution.  This happens in the elf_machine_load_address() 
function.

To maintain compatibility with these versions, we could change the test 
in do_page_fault() to include VM_READ as well as VM_EXEC on targets that 
don't have a separate exec-bit in hardware (are there any powerpc mmus 
that do?).  However, Segher suggested on IRC that we may want to drop 
compatibility with those old versions of glibc, and that I should seek 
your input.

Personally, I'd rather stick the VM_READ in there, partially for selfish 
reasons (our root filesystems are based on older glibcs), and because it 
seems a little too soon to deprecate glibc 2.3, but also because in the 
absence of hardware support, the VM_EXEC check will be nondeterministic, 
kicking in only when the first fault for a page is to execute.

-Scott

[1] It's possible that there are other instances of this in 2.4 and that 
the actual version is newer; I ran into obnoxious cross compilation 
issues trying to try it.  However,

<rant>
Glibc already has target-specific code/headers; if you need to know 
something that you'd otherwise need a runs-on-the-target autoconf test 
for, why not just stick it in such a target-specific header?  In this 
case, it was trying to figure out the size of "long double".
</rant>

On Thu, 2007-07-05 at 16:55 -0500, Scott Wood wrote:

As revealed by the recent "Prevent data exception in kernel space" 
patch, versions of glibc prior to 2.4[1] assume that, on powerpc32, they 
can execute out of any readable mapping, regardless of whether it is 
marked for execution.  This happens in the elf_machine_load_address() 
function.

To maintain compatibility with these versions, we could change the test 
in do_page_fault() to include VM_READ as well as VM_EXEC on targets that 
don't have a separate exec-bit in hardware (are there any powerpc mmus 
that do?).  However, Segher suggested on IRC that we may want to drop 
compatibility with those old versions of glibc, and that I should seek 
your input.

Personally, I'd rather stick the VM_READ in there, partially for selfish 
reasons (our root filesystems are based on older glibcs), and because it 
seems a little too soon to deprecate glibc 2.3, but also because in the 
absence of hardware support, the VM_EXEC check will be nondeterministic, 
kicking in only when the first fault for a page is to execute.

-Scott

[1] It's possible that there are other instances of this in 2.4 and that 
the actual version is newer; I ran into obnoxious cross compilation 
issues trying to try it.  However,

<rant>
Glibc already has target-specific code/headers; if you need to know 
something that you'd otherwise need a runs-on-the-target autoconf test 
for, why not just stick it in such a target-specific header?  In this 
case, it was trying to figure out the size of "long double".
</rant>
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Since may be a security problem (non-exec mappings being executed), I
really believe we don't want to keep the compatibility.


Regards,


Rodrigo (BSDaemon).

-- 
Rodrigo Rubira Branco
Software Engineer 
Advanced Linux Response Team (ALRT) / Linux on Power Toolchain
IBM Linux Technology Center (IBM/LTC)
rrbranco@br.ibm.com

GPG KeyID: 1FCEDEA1

On Thu, 2007-07-05 at 16:55 -0500, Scott Wood wrote:

As revealed by the recent "Prevent data exception in kernel space" 
patch, versions of glibc prior to 2.4[1] assume that, on powerpc32, they 
can execute out of any readable mapping, regardless of whether it is 
marked for execution.  This happens in the elf_machine_load_address() 
function.

Umm, are you sure about this? The "prevent data exception in kernel
space" patch came from a test program I had that attempted to execute a
page with /no permissions/ at all!

I know that I used to have problems with mono making this assumption but
these have since been fixed; however if I understand you correctly then
you can always pre-fault the page by a read and then execute it so I
don't see the point in not doing the change you suggest.

johannes

As revealed by the recent "Prevent data exception in kernel
space" patch, versions of glibc prior to 2.4[1] assume that,

Are you sure that this hasn't been fixed somewhere in the 2.3
series, too?

on powerpc32, they can execute out of any readable mapping,
regardless of whether it is marked for execution.  This happens
in the elf_machine_load_address() function.

To maintain compatibility with these versions, we could change
the test in do_page_fault() to include VM_READ as well as VM_EXEC
on targets that don't have a separate exec-bit in hardware (are
there any powerpc mmus that do?).  However, Segher suggested on
IRC that we may want to drop compatibility with those old versions
of glibc, and that I should seek your input.

"drop compatibility" -- this sounds worse than it is, this is all
about supporting a badly broken application.

I wonder how this works with 32-bit userland on a 64-bit kernel,
or with older kernels.  With arch/ppc/ an exec fault would do
the read checks IIRC, maybe we need to change to that.

Personally, I'd rather stick the VM_READ in there, partially for
selfish reasons (our root filesystems are based on older glibcs),
and because it seems a little too soon to deprecate glibc 2.3,

Oh I don't know, can't we just deprecate glibc completely?  ;-)

but also because in the absence of hardware support, the VM_EXEC
check will be nondeterministic, kicking in only when the first
fault for a page is to execute.

I don't think that is a big concern.

[1] It's possible that there are other instances of this in 2.4 and  
that the actual version is newer; I ran into obnoxious cross  
compilation issues trying to try it.  However,

<rant>"Trying to try it" sounds like compiling glibc, indeed.</rant>

<rant>
Glibc already has target-specific code/headers; if you need to know  
something that you'd otherwise need a runs-on-the-target autoconf  
test for, why not just stick it in such a target-specific header?   
In this case, it was trying to figure out the size of "long double".
</rant>

You can do a test like this by cross-compiling some code, and looking
at the size of the symbol in the resulting object file.  Well that's
all off-topic here.


Segher

quoted

As revealed by the recent "Prevent data exception in kernel space"
patch, versions of glibc prior to 2.4[1] assume that, on  
powerpc32, they
can execute out of any readable mapping, regardless of whether it is
marked for execution.  This happens in the elf_machine_load_address()
function.

Umm, are you sure about this? The "prevent data exception in kernel
space" patch came from a test program I had that attempted to  
execute a
page with /no permissions/ at all!

I haven't looked at the code path in detail, but I believe Scott's
analysis is correct.  The kernel would merrily let a program run
code from a page without execute permission (so also from a page
without any permissions at all); not anymore, after my patch.

Programs relying on this behaviour are obviously buggy, but the
problem is that one of these broken programs is glibc, at least
some not-all-that-new but also not-all-that-old versions.

I know that I used to have problems with mono making this  
assumption but
these have since been fixed; however if I understand you correctly  
then
you can always pre-fault the page by a read and then execute it so I
don't see the point in not doing the change you suggest.

Too many negatives, I don't see which way you're arguing :-)

I think you're saying to treat read access as including execute
access?  I believe that would be too permissive here.

Anyway, let's first decide what is the right thing to do, and
only then look at the code ;-)


Segher

On Fri, 2007-07-06 at 15:36 +0200, Segher Boessenkool wrote:

quoted

Umm, are you sure about this? The "prevent data exception in kernel
space" patch came from a test program I had that attempted to  
execute a
page with /no permissions/ at all!

I haven't looked at the code path in detail, but I believe Scott's
analysis is correct.  The kernel would merrily let a program run
code from a page without execute permission (so also from a page
without any permissions at all); not anymore, after my patch.

Programs relying on this behaviour are obviously buggy, but the
problem is that one of these broken programs is glibc, at least
some not-all-that-new but also not-all-that-old versions.

Ok I can see how your patch changes that if the page is readable but not
executable and hasn't been faulted in yet, then executing it will kill
the program. However, reading first (prefaulting) and then executing
code on the page will not kill the program.

Too many negatives, I don't see which way you're arguing :-)

Heh.

I think you're saying to treat read access as including execute
access?  I believe that would be too permissive here.

Yeah well what I was trying to say is that there's no point in having
read without execute if prefaulting the page can get you around that.

johannes

On Thu, 2007-07-05 at 16:55 -0500, Scott Wood wrote:

To maintain compatibility with these versions, we could change the test 
in do_page_fault() to include VM_READ as well as VM_EXEC on targets that 
don't have a separate exec-bit in hardware (are there any powerpc mmus 
that do?).  

64-bit implementations since POWER4 have separate exec permissions,
don't they? So any userspace which tries to execute non-executable pages
is already broken when running on a ppc64 machine; I wouldn't worry too
much about letting it break on ppc32 too.

We already use vDSO signal trampolines even on the 32-bit kernel, right?

-- 
dwmw2

On Fri, Jul 06, 2007 at 03:24:20PM +0200, Segher Boessenkool wrote:

quoted

Personally, I'd rather stick the VM_READ in there, partially for
selfish reasons (our root filesystems are based on older glibcs),
and because it seems a little too soon to deprecate glibc 2.3,

Oh I don't know, can't we just deprecate glibc completely?  ;-)

I wish. :-)

quoted

but also because in the absence of hardware support, the VM_EXEC
check will be nondeterministic, kicking in only when the first
fault for a page is to execute.

I don't think that is a big concern.

Well, it means that leaving VM_READ out of the check (except where the
hardware PTE has an exec bit) isn't really buying us anything
security-wise (especially since the primary reason for no-exec protection
is to avoid code injections via stack overflow, and those pages will
usually already be present), so it doesn't hurt much to let things keep
working.

At the least, I'd like it to keep working for a few more kernel releases
(with a warning printed when a VM_EXEC-only test would have failed), so
people have time to upgrade glibc.

-Scott

On Fri, 2007-07-06 at 10:42 -0400, David Woodhouse wrote:

On Thu, 2007-07-05 at 16:55 -0500, Scott Wood wrote:

quoted

To maintain compatibility with these versions, we could change the test 
in do_page_fault() to include VM_READ as well as VM_EXEC on targets that 
don't have a separate exec-bit in hardware (are there any powerpc mmus 
that do?).  

64-bit implementations since POWER4 have separate exec permissions,
don't they? So any userspace which tries to execute non-executable pages
is already broken when running on a ppc64 machine; I wouldn't worry too
much about letting it break on ppc32 too.

We already use vDSO signal trampolines even on the 32-bit kernel, right?

The problem was with old glibc 32 bits which used to jump to an
instruction in the .got iirc that wasn't in an executable section

Ben.

Well, it means that leaving VM_READ out of the check (except where the
hardware PTE has an exec bit) isn't really buying us anything
security-wise (especially since the primary reason for no-exec protection
is to avoid code injections via stack overflow, and those pages will
usually already be present), so it doesn't hurt much to let things keep
working.

At the least, I'd like it to keep working for a few more kernel releases
(with a warning printed when a VM_EXEC-only test would have failed), so
people have time to upgrade glibc.

I agree. Care to send a patch ? :-0

Ben.