|
Message-ID: <20161102174031.GI5749@port70.net> Date: Wed, 2 Nov 2016 18:40:31 +0100 From: Szabolcs Nagy <nsz@...t70.net> To: "LeMay, Michael" <michael.lemay@...el.com> Cc: "musl@...ts.openwall.com" <musl@...ts.openwall.com> Subject: Re: [RFC PATCH v2 2/4] support SafeStack in init and threading * LeMay, Michael <michael.lemay@...el.com> [2016-11-02 09:56:14 -0700]: > On 11/1/2016 16:52, Szabolcs Nagy wrote: > > * LeMay, Michael <michael.lemay@...el.com> [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_*.
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.