From owner-svn-src-all@FreeBSD.ORG Fri Oct 14 18:53:11 2011 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A0EB61065678; Fri, 14 Oct 2011 18:53:11 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mail.zoral.com.ua (mx0.zoral.com.ua [91.193.166.200]) by mx1.freebsd.org (Postfix) with ESMTP id E742F8FC12; Fri, 14 Oct 2011 18:53:10 +0000 (UTC) Received: from alf.home (alf.kiev.zoral.com.ua [10.1.1.177]) by mail.zoral.com.ua (8.14.2/8.14.2) with ESMTP id p9EIr7s8045050 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 14 Oct 2011 21:53:07 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: from alf.home (kostik@localhost [127.0.0.1]) by alf.home (8.14.5/8.14.5) with ESMTP id p9EIr7um039821; Fri, 14 Oct 2011 21:53:07 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by alf.home (8.14.5/8.14.5/Submit) id p9EIr6tC039820; Fri, 14 Oct 2011 21:53:06 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: alf.home: kostik set sender to kostikbel@gmail.com using -f Date: Fri, 14 Oct 2011 21:53:06 +0300 From: Kostik Belousov To: Garrett Cooper Message-ID: <20111014185306.GQ1511@deviant.kiev.zoral.com.ua> References: <201110131620.p9DGKAM2022926@svn.freebsd.org> <20111013190943.GM1511@deviant.kiev.zoral.com.ua> <201110131707.14466.jhb@freebsd.org> <20111013225030.GA75054@zim.MIT.EDU> <20111014182443.GP1511@deviant.kiev.zoral.com.ua> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="ECruMGViw3NXTj6O" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: clamav-milter 0.95.2 at skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-3.3 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00, DNS_FROM_OPENWHOIS autolearn=no version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on skuns.kiev.zoral.com.ua Cc: Marcel Moolenaar , John Baldwin , svn-src-all@freebsd.org, Marcel Moolenaar , src-committers@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r226343 - head/sys/vm X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 14 Oct 2011 18:53:11 -0000 --ECruMGViw3NXTj6O Content-Type: text/plain; charset=koi8-r Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Oct 14, 2011 at 11:30:47AM -0700, Garrett Cooper wrote: > On Fri, Oct 14, 2011 at 11:24 AM, Kostik Belousov w= rote: > > 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 ti= me > >> > >> in the past the ABI changed: at what precise time does "supported > >> > >> by hardware mean" and how does that tie to a major FreeBSD versio= n? > >> > >> > >> > >> 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 tha= t the > >> > > jdk did not work. =9AThat will be true for any i386 PAE kernel bac= k 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. =9AIn > >> Linux, the flag can be set using an ld option or the execstack(8) > >> command. =9AThat 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 a= re > > 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/freebsd= 32/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 freebs= d32_mprotect_args *uap) > > =9A =9A =9A =9Aap.len =3D uap->len; > > =9A =9A =9A =9Aap.prot =3D uap->prot; > > =9A#if defined(__amd64__) || defined(__ia64__) > > - =9A =9A =9A if (ap.prot & PROT_READ) > > + =9A =9A =9A if (i386_read_exec && (ap.prot & PROT_READ) !=3D 0) > > =9A =9A =9A =9A =9A =9A =9A =9Aap.prot |=3D PROT_EXEC; > > =9A#endif > > =9A =9A =9A =9Areturn (sys_mprotect(td, &ap)); > > @@ -536,7 +536,7 @@ freebsd32_mmap(struct thread *td, struct freebsd32_= mmap_args *uap) > > =9A#endif > > > > =9A#if defined(__amd64__) || defined(__ia64__) > > - =9A =9A =9A if (prot & PROT_READ) > > + =9A =9A =9A if (i386_read_exec && (prot & PROT_READ)) > > =9A =9A =9A =9A =9A =9A =9A =9Aprot |=3D PROT_EXEC; > > =9A#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 =3D 0; > > =9ASYSCTL_INT(_debug, OID_AUTO, __elfN(legacy_coredump), CTLFLAG_RW, > > =9A =9A &elf_legacy_coredump, 0, ""); > > > > -static int __elfN(nxstack) =3D 0; > > +static int __elfN(nxstack) =3D > > +#if defined(__amd64__) || defined(__powerpc__) /* both 64 and 32 bit */ > > + =9A =9A =9A 1; > > +#else > > + =9A =9A =9A 0; > > +#endif > > =9ASYSCTL_INT(__CONCAT(_kern_elf, __ELF_WORD_SIZE), OID_AUTO, > > =9A =9A nxstack, CTLFLAG_RW, &__elfN(nxstack), 0, > > =9A =9A __XSTRING(__CONCAT(ELF, __ELF_WORD_SIZE)) ": enable non-executa= ble stack"); > > > > +#if __ELF_WORD_SIZE =3D=3D 32 > > +#if defined(__amd64__) || defined(__ia64__) > > +int i386_read_exec =3D 0; > > +SYSCTL_INT(_kern_elf32, OID_AUTO, read_exec, CTLFLAG_RW, &i386_read_ex= ec, 0, > > + =9A =9A"enable execution from readable segments"); > > +#endif > > +#endif > > + > > =9Astatic Elf_Brandinfo *elf_brand_list[MAX_BRANDS]; > > > > =9A#define =9A =9A =9A =9Atrunc_page_ps(va, ps) =9A ((va) & ~(ps - 1)) > > @@ -1666,7 +1679,7 @@ __elfN(trans_prot)(Elf_Word flags) > > =9A =9A =9A =9A =9A =9A =9A =9Aprot |=3D VM_PROT_READ; > > =9A#if __ELF_WORD_SIZE =3D=3D 32 > > =9A#if defined(__amd64__) || defined(__ia64__) > > - =9A =9A =9A if (flags & PF_R) > > + =9A =9A =9A if (i386_read_exec && (flags & PF_R)) > > =9A =9A =9A =9A =9A =9A =9A =9Aprot |=3D VM_PROT_EXECUTE; > > =9A#endif > > =9A#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; > > =9Aextern struct sysent sysent[]; > > =9Aextern const char *syscallnames[]; > > > > +#if defined(__amd64__) || defined(__ia64__) > > +extern int i386_read_exec; > > +#endif > > + > > =9A#define =9A =9A =9A =9ANO_SYSCALL (-1) > > > > =9Astruct 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) > > =9A =9A =9A =9A =9A =9A =9A =9Aprot =3D VM_PROT_RW; > > =9A#ifdef COMPAT_FREEBSD32 > > =9A#if defined(__amd64__) || defined(__ia64__) > > - =9A =9A =9A =9A =9A =9A =9A if (SV_PROC_FLAG(td->td_proc, SV_ILP32)) > > + =9A =9A =9A =9A =9A =9A =9A if (i386_read_exec && SV_PROC_FLAG(td->td= _proc, SV_ILP32)) > > =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9Aprot |=3D VM_PROT_EXECUT= E; > > =9A#endif > > =9A#endif >=20 > 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 --ECruMGViw3NXTj6O Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.18 (FreeBSD) iEYEARECAAYFAk6YhRIACgkQC3+MBN1Mb4h44wCgvaoSL05myzO53pEa1LcERCAx YuAAmgN6sYsD3D4mOGwrmBXbnsvKO7em =/h2b -----END PGP SIGNATURE----- --ECruMGViw3NXTj6O--