From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/10695 Path: news.gmane.org!.POSTED!not-for-mail From: Szabolcs Nagy Newsgroups: gmane.linux.lib.musl.general Subject: Re: [RFC PATCH v2 2/4] support SafeStack in init and threading Date: Wed, 2 Nov 2016 18:40:31 +0100 Message-ID: <20161102174031.GI5749@port70.net> References: <390CE752059EB848A71F4F676EBAB76D3AC2636A@ORSMSX114.amr.corp.intel.com> <20161101235219.GH5749@port70.net> <456a88bf-50bb-479d-11c5-bd82ce116e67@intel.com> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: blaine.gmane.org 1478108465 20202 195.159.176.226 (2 Nov 2016 17:41:05 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Wed, 2 Nov 2016 17:41:05 +0000 (UTC) User-Agent: Mutt/1.6.0 (2016-04-01) Cc: "musl@lists.openwall.com" To: "LeMay, Michael" Original-X-From: musl-return-10708-gllmg-musl=m.gmane.org@lists.openwall.com Wed Nov 02 18:41:00 2016 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.84_2) (envelope-from ) id 1c1zWl-0002zv-1Y for gllmg-musl@m.gmane.org; Wed, 02 Nov 2016 18:40:43 +0100 Original-Received: (qmail 11735 invoked by uid 550); 2 Nov 2016 17:40:44 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 11716 invoked from network); 2 Nov 2016 17:40:43 -0000 Mail-Followup-To: "LeMay, Michael" , "musl@lists.openwall.com" Content-Disposition: inline In-Reply-To: <456a88bf-50bb-479d-11c5-bd82ce116e67@intel.com> Xref: news.gmane.org gmane.linux.lib.musl.general:10695 Archived-At: * LeMay, Michael [2016-11-02 09:56:14 -0700]: > On 11/1/2016 16:52, Szabolcs Nagy wrote: > > * LeMay, Michael [2016-10-28 20:00:24 +0000]: > ... > > > +#if SAFE_STACK > > > +void __init_unsafe_stack(void); > > > + > > > +__attribute__((no_sanitize("safe-stack"))) > > > +#endif > > > void __init_libc(char **envp, char *pn) > > > { > > > size_t i, *auxv, aux[AUX_CNT] = { 0 }; > > > @@ -36,6 +41,9 @@ void __init_libc(char **envp, char *pn) > > > } > > > __init_tls(aux); > > > +#if SAFE_STACK > > > + __init_unsafe_stack(); > > > +#endif > > > __init_ssp((void *)aux[AT_RANDOM]); > > > if (aux[AT_UID]==aux[AT_EUID] && aux[AT_GID]==aux[AT_EGID] > > > @@ -63,10 +71,18 @@ static void libc_start_init(void) > > > weak_alias(libc_start_init, __libc_start_init); > > > +#if SAFE_STACK > > > +void __preinit_unsafe_stack(void); > > > + > > > +__attribute__((no_sanitize("safe-stack"))) > > > +#endif > > > int __libc_start_main(int (*main)(int,char **,char **), int argc, char **argv) > > > { > > > char **envp = argv+argc+1; > > > +#if SAFE_STACK > > > + __preinit_unsafe_stack(); > > > +#endif > > > __init_libc(envp, argv[0]); > > > __libc_start_init(); > > > diff --git a/src/internal/pthread_impl.h b/src/internal/pthread_impl.h > > > index 3890bb5..2b4fc30 100644 > > in general i'd avoid using ifdefs if possible (e.g. have > > the function calls there unconditionally just make them > > dummy when there is no safe stack), but i don't know how > > to deal with the attribute: can't you whitelist this file > > in the makefile so it's not instrumented? > Yes, I can do that. i'm actually not sure about this, wait for others to comment musl uses the directory tree for target specific behaviour (target specific version of foo.c is arch/foo.*), but in case of SAFE_STACK that's probably not reasonable. > > > +/* There are no checks for overflows past the end of this stack buffer. It must > > > + * be allocated with adequate space to meet the requirements of all of the code > > > + * that runs prior to __init_unsafe_stack allocating a new unsafe stack. This > > > + * buffer is not used after that. */ > > > +static uint8_t preinit_us[PAGE_SIZE]; > > note that PAGE_SIZE is not compile time constant expression > > on some targets see internal/libc.h (but on x86 this should > > work). > I chose to use PAGE_SIZE simply to provide ample space. It does not > actually need to match the machine's page size. This file is > architecture-independent, so I'll replace PAGE_SIZE with a numeric constant. > > and i'd use char type. > Sure, I'll update that. For my future reference, can you please tell me why > char is preferred? this is just style, others might disagree, but (unsigned) char has special properties in c (it can alias any object, its pointer has the same representation as void*, it has size 1) that is not guaranteed for uint8_t in principle and it is always available without header includes. (here only the size 1 property is used which holds for uint8_t too in posix so both are fine.) > > > + if (pthread_getattr_np(self, &attr) != 0) > > > + a_crash(); > > this may have significant startup overhead because determining > > the main stack size is not optimized. > I could use a constant stack size here instead. However, this will be > needed for supporting a separate stack segment, since it is used to > determine the proper limit for the DS and ES segments. ok the pthread_* symbol should be moved to the reserved namespace (__pthread_*) since this code will get static linked into iso c conforming code which is allowed to redefine pthread_*.