svn commit: r226343 - head/sys/vm

[Kostik Belousov]

On Fri, Oct 14, 2011 at 11:30:47AM -0700, Garrett Cooper wrote:
> On Fri, Oct 14, 2011 at 11:24 AM, Kostik Belousov <kostikbel at gmail.com> wrote:
> > On Thu, Oct 13, 2011 at 06:50:30PM -0400, David Schultz wrote:
> >> On Thu, Oct 13, 2011, Marcel Moolenaar wrote:
> >> >
> >> > On Oct 13, 2011, at 2:07 PM, John Baldwin wrote:
> >> > >>
> >> > >> That's really besides the point. ABI changes are made deliberately
> >> > >> and ABIs must be well-documented for anyone to adhere to it. You
> >> > >> can't post hoc wave your hand and say that at some unspecified time
> >> > >> in the past the ABI changed: at what precise time does "supported
> >> > >> by hardware mean" and how does that tie to a major FreeBSD version?
> >> > >>
> >> > >> Point in case: the JDK 1.4.x still works on FreeBSD 9.x (i386), so
> >> > >> the ABI really hasn't changed at all in that respect.
> >> > >
> >> > > I think if you booted a FreeBSD 9.x i386 PAE kernel you'd find that the
> >> > > jdk did not work.  That will be true for any i386 PAE kernel back to
> >> > > when PG_NX support was introduced.
> >> >
> >> > That's bad.
> > After more thought about the issue, I do not agree with you.
> > Elf specification says about the PF_R flag that only read permission
> > for the memory image of the segment is required, but read and execute
> > is allowed.
> >
> > In other words, it is a bug in the old jre, which is further confirmed
> > by the fact that later jres run with non-executable head.
> >
> >>
> >> Recent binutils support a PT_GNU_HEAP flag in the ELF header that
> >> controls whether heap allocations are executable by default.  In
> >> Linux, the flag can be set using an ld option or the execstack(8)
> >> command.  That seems like a better way to go than breaking old
> >> JVMs or disabling the security feature.
> >
> > No, recent binutils do not support PT_GNU_HEAD. git grep -e PT_GNU_HEAD
> > on the up to date checkout of binutils head gives zero matches. There are
> > indeed PT_GNU_HEAP patches floating around from some hardening projects.
> >
> > There is indeed PT_GNU_STACK, which we do support.
> >
> > I want to commit the following refinement:
> >
> > diff --git a/sys/compat/freebsd32/freebsd32_misc.c b/sys/compat/freebsd32/freebsd32_misc.c
> > index 6638ec8..fc2932b 100644
> > --- a/sys/compat/freebsd32/freebsd32_misc.c
> > +++ b/sys/compat/freebsd32/freebsd32_misc.c
> > @@ -445,7 +445,7 @@ freebsd32_mprotect(struct thread *td, struct freebsd32_mprotect_args *uap)
> >        ap.len = uap->len;
> >        ap.prot = uap->prot;
> >  #if defined(__amd64__) || defined(__ia64__)
> > -       if (ap.prot & PROT_READ)
> > +       if (i386_read_exec && (ap.prot & PROT_READ) != 0)
> >                ap.prot |= PROT_EXEC;
> >  #endif
> >        return (sys_mprotect(td, &ap));
> > @@ -536,7 +536,7 @@ freebsd32_mmap(struct thread *td, struct freebsd32_mmap_args *uap)
> >  #endif
> >
> >  #if defined(__amd64__) || defined(__ia64__)
> > -       if (prot & PROT_READ)
> > +       if (i386_read_exec && (prot & PROT_READ))
> >                prot |= PROT_EXEC;
> >  #endif
> >
> > diff --git a/sys/kern/imgact_elf.c b/sys/kern/imgact_elf.c
> > index 669c652..9970386 100644
> > --- a/sys/kern/imgact_elf.c
> > +++ b/sys/kern/imgact_elf.c
> > @@ -118,11 +118,24 @@ static int elf_legacy_coredump = 0;
> >  SYSCTL_INT(_debug, OID_AUTO, __elfN(legacy_coredump), CTLFLAG_RW,
> >     &elf_legacy_coredump, 0, "");
> >
> > -static int __elfN(nxstack) = 0;
> > +static int __elfN(nxstack) =
> > +#if defined(__amd64__) || defined(__powerpc__) /* both 64 and 32 bit */
> > +       1;
> > +#else
> > +       0;
> > +#endif
> >  SYSCTL_INT(__CONCAT(_kern_elf, __ELF_WORD_SIZE), OID_AUTO,
> >     nxstack, CTLFLAG_RW, &__elfN(nxstack), 0,
> >     __XSTRING(__CONCAT(ELF, __ELF_WORD_SIZE)) ": enable non-executable stack");
> >
> > +#if __ELF_WORD_SIZE == 32
> > +#if defined(__amd64__) || defined(__ia64__)
> > +int i386_read_exec = 0;
> > +SYSCTL_INT(_kern_elf32, OID_AUTO, read_exec, CTLFLAG_RW, &i386_read_exec, 0,
> > +    "enable execution from readable segments");
> > +#endif
> > +#endif
> > +
> >  static Elf_Brandinfo *elf_brand_list[MAX_BRANDS];
> >
> >  #define        trunc_page_ps(va, ps)   ((va) & ~(ps - 1))
> > @@ -1666,7 +1679,7 @@ __elfN(trans_prot)(Elf_Word flags)
> >                prot |= VM_PROT_READ;
> >  #if __ELF_WORD_SIZE == 32
> >  #if defined(__amd64__) || defined(__ia64__)
> > -       if (flags & PF_R)
> > +       if (i386_read_exec && (flags & PF_R))
> >                prot |= VM_PROT_EXECUTE;
> >  #endif
> >  #endif
> > diff --git a/sys/sys/sysent.h b/sys/sys/sysent.h
> > index 6a4b485..3694ceb 100644
> > --- a/sys/sys/sysent.h
> > +++ b/sys/sys/sysent.h
> > @@ -151,6 +152,10 @@ extern struct sysentvec null_sysvec;
> >  extern struct sysent sysent[];
> >  extern const char *syscallnames[];
> >
> > +#if defined(__amd64__) || defined(__ia64__)
> > +extern int i386_read_exec;
> > +#endif
> > +
> >  #define        NO_SYSCALL (-1)
> >
> >  struct module;
> > diff --git a/sys/vm/vm_unix.c b/sys/vm/vm_unix.c
> > index d4ea3b7..253ab77 100644
> > --- a/sys/vm/vm_unix.c
> > +++ b/sys/vm/vm_unix.c
> > @@ -141,7 +141,7 @@ sys_obreak(td, uap)
> >                prot = VM_PROT_RW;
> >  #ifdef COMPAT_FREEBSD32
> >  #if defined(__amd64__) || defined(__ia64__)
> > -               if (SV_PROC_FLAG(td->td_proc, SV_ILP32))
> > +               if (i386_read_exec && SV_PROC_FLAG(td->td_proc, SV_ILP32))
> >                        prot |= VM_PROT_EXECUTE;
> >  #endif
> >  #endif
> 
>     Would you want to be able to toggle this feature at runtime after
> you've fully booted your kernel? It seems like it would be kind of
Yes, I do.
> dangerous to implement it on a system wide basis with part of the
> pages NX and the others not NX -- in particular if multiple processes
> are modifying the stack simultaneously.
>     Also, would the sysctl be atomic enough for this to be a good idea?
It is atomic enough.

>     Personally I think that it would be better if this was a tunable
> (and a readonly sysctl so folks could programmatically detect whether
> or not NX was on), but that might just be me..
> Thanks,
> -Garrett
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/svn-src-all/attachments/20111014/0ccb8a8d/attachment.pgp