[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <456a88bf-50bb-479d-11c5-bd82ce116e67@intel.com>
Date: Wed, 2 Nov 2016 09:56:14 -0700
From: "LeMay, Michael" <michael.lemay@...el.com>
To: Szabolcs Nagy <nsz@...t70.net>
Cc: "musl@...ts.openwall.com" <musl@...ts.openwall.com>
Subject: Re: [RFC PATCH v2 2/4] support SafeStack in init and threading
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.
...
>> +/* 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?
>> +
>> +__attribute__((no_sanitize("safe-stack"), __visibility__("hidden")))
> can this file be whitelisted to remain uninstrumented?
Yes, I'll do that.
>
>> +void __init_unsafe_stack(void)
>> +{
>> + void *stack_base;
>> + size_t stack_size;
>> + pthread_attr_t attr;
>> + struct pthread *self;
>> +
>> + if (unsafe_stack_ptr_inited)
>> + return;
>> +
>> + self = __pthread_self();
>> +
>> + /* Set the unsafe stack pointer in the current TCB to the statically-allocated
>> + * unsafe stack, since some of the subroutines invoked below may use the
>> + * unsafe stack. */
>> + self->unsafe_stack_ptr = preinit_us + sizeof(preinit_us);
> is there an alignment requirement for this pointer?
No.
>
>> +
>> + 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.
>
>> +
>> + if (pthread_attr_getstack(&attr, &stack_base, &stack_size) != 0)
>> + a_crash();
>> +
>> + stack_size *= 2;
>> +
...
>>
>> -_Noreturn void __pthread_exit(void *result)
>> +_Noreturn void __pthread_exit_generic(void *result)
> i dont see why this is needed.
Oops, I had implemented an alternative __pthread_exit routine to
deallocate the safe stack before invoking __pthread_exit_generic, but it
must have gotten lost during a rebase.
>
...
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.
