Re: [RFC PATCH v9 for 4.15 01/14] Restartable sequences system call

[Mathieu Desnoyers <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>]

----- On Oct 13, 2017, at 2:17 PM, Andy Lutomirski luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org wrote:

> On Fri, Oct 13, 2017 at 10:53 AM, Florian Weimer <fweimer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> On 10/13/2017 07:24 PM, Andy Lutomirski wrote:
>>>
>>> On Fri, Oct 13, 2017 at 7:27 AM, Mathieu Desnoyers
>>> <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org> wrote:
>>>>
>>>> ----- On Oct 13, 2017, at 9:56 AM, Florian Weimer fweimer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
>>>> wrote:
>>>>
>>>>> On 10/13/2017 03:40 PM, Mathieu Desnoyers wrote:
>>>>>>
>>>>>> The proposed ABI does not require to store any function pointer. For a
>>>>>> given
>>>>>> rseq_finish() critical section, pointers to specific instructions
>>>>>> (within a
>>>>>> function) are emitted at link-time into a struct rseq_cs:
>>>>>>
>>>>>> struct rseq_cs {
>>>>>>           RSEQ_FIELD_u32_u64(start_ip);
>>>>>>           RSEQ_FIELD_u32_u64(post_commit_ip);
>>>>>>           RSEQ_FIELD_u32_u64(abort_ip);
>>>>>>           uint32_t flags;
>>>>>> } __attribute__((aligned(4 * sizeof(uint64_t))));
>>>>>>
>>>>>> Then, at runtime, the fast-path stores the address of that struct
>>>>>> rseq_cs
>>>>>> into the TLS struct rseq "rseq_cs" field.
>>>>>>
>>>>>> So all we store at runtime is a pointer to data, not a pointer to
>>>>>> functions.
>>>>>>
>>>>>> But you seem to hint that having a pointer to data containing pointers
>>>>>> to code
>>>>>> may still be making it easier for exploit writers. Can you elaborate on
>>>>>> the
>>>>>> scenario ?
>>>>>
>>>>>
>>>>> I'm concerned that the exploit writer writes a totally made up struct
>>>>> rseq_cs object into writable memory, along with function pointers, and
>>>>> puts the address of that in to the rseq_cs field.
>>>>>
>>>>> This would be comparable to how C++ vtable pointers are targeted
>>>>> (including those in the glibc libio implementation of stdio streams).
>>>>>
>>>>> Does this answer your questions?
>>>>
>>>>
>>>> Yes, it does. How about we add a "canary" field to the TLS struct rseq,
>>>> e.g.:
>>>>
>>>> struct rseq {
>>>>          union rseq_cpu_event u;
>>>>          RSEQ_FIELD_u32_u64(rseq_cs);  -> pointer to struct rseq_cs
>>>>          uint32_t flags;
>>>>          uint32_t canary;   -> 32 low bits of rseq_cs ^ canary_mask
>>>> };
>>>>
>>>> We could then add a "uint32_t canary_mask" argument to sys_rseq, e.g.:
>>>>
>>>> SYSCALL_DEFINE3(rseq, struct rseq __user *, rseq, uint32_t, canary_mask,
>>>> int, flags);
>>>>
>>>> So a thread which does not care about hardening would simply register its
>>>> struct rseq TLS with a canary mask of "0". Nothing changes on the
>>>> fast-path.
>>>>
>>>> A thread belonging to a process that cares about hardening could use a
>>>> random
>>>> value as canary, and pass it as canary_mask argument to the syscall. The
>>>> fast-path could then set the struct rseq "canary" value to
>>>> (32-low-bits of rseq_cs) ^ canary_mask just surrounding the critical
>>>> section,
>>>> and set it back to 0 afterward.
>>>>
>>>> In the kernel, whenever the rseq_cs pointer would be loaded, its 32 low
>>>> bits
>>>> would be checked to match (canary ^ canary_mask). If it differs, then the
>>>> kernel kills the process with SIGSEGV.
>>>>
>>>> Would that take care of your concern ?
>>>>
>>>
>>> I would propose a slightly different solution: have the kernel verify
>>> that it jumps to a code sequence that occurs just after some
>>> highly-unlikely magic bytes in the text *and* that those bytes have
>>> some signature that matches a signature in the struct rseq that's
>>> passed in.
>>
>>
>> And the signature is fixed at the time of the rseq syscall?
> 
> The point of the signature is to prevent an rseq landing pad from
> being used out of context.  Actually getting the details right might
> be tricky.

So my understanding is that we want to prevent an attacker that
controls the stack to easily use rseq to trick the kernel into
branching into an arbitrary pre-existing executable address in
the process.

I like the idea of putting a signature just before the abort_ip
landing address and having it checked by the kernel. We could start
by using a fixed hardcoded signature for now, and pass the
signature value to the kernel when registering rseq. This would
eventually allow a process to use a randomized signature if we
figure out it's needed in the future.

I don't see how placing this signature in struct rseq TLS area
is a good idea: an attacker could then just overwrite that value
so it matches whatever code is before the branch target it wishes
to branch to.

I also don't get how having the signature in the struct rseq_cs
(restartable sequence descriptor) alongside with start/end/abort
ip can be useful. Typically, an attacker would put its fake structure
either on the stack, in data, or in rw memory, and make sure it
uses the right signature in there. In the end, we don't really care
whether the user ends up controlling the content of a struct rseq_cs,
what we really care about is that it does not make the kernel branch
to a pre-existing executable code address of its choosing.

So having the kernel validate a signature placed just before the
abort_ip should be enough for hardening purposes.

Thoughts ?

Thanks,

Mathieu


> 
>>
>> Yes, that would be far more reliable.
>>
>> Thanks,
>> Florian
> 
> 
> 
> --
> Andy Lutomirski
> AMA Capital Management, LLC

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com