Message-ID: <CALCETrVWZxC=mT9p7HTrAwcAdMzaxwa=A-O0uQt79qy1Cpky_g@mail.gmail.com>
Date:   Fri, 13 Oct 2017 11:17:04 -0700
From:   Andy Lutomirski <luto@...capital.net>
To:     Florian Weimer <fweimer@...hat.com>
Cc:     Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
        Boqun Feng <boqun.feng@...il.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Paul Turner <pjt@...gle.com>, Andrew Hunter <ahh@...gle.com>,
        Dave Watson <davejwatson@...com>,
        Josh Triplett <josh@...htriplett.org>,
        Will Deacon <will.deacon@....com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Andi Kleen <andi@...stfloor.org>, Chris Lameter <cl@...ux.com>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>, Ben Maurer <bmaurer@...com>,
        rostedt <rostedt@...dmis.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Russell King <linux@....linux.org.uk>,
        Catalin Marinas <catalin.marinas@....com>,
        Michael Kerrisk <mtk.manpages@...il.com>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        linux-api <linux-api@...r.kernel.org>
Subject: Re: [RFC PATCH v9 for 4.15 01/14] Restartable sequences system call

On Fri, Oct 13, 2017 at 10:53 AM, Florian Weimer <fweimer@...hat.com> wrote:
> On 10/13/2017 07:24 PM, Andy Lutomirski wrote:
>>
>> On Fri, Oct 13, 2017 at 7:27 AM, Mathieu Desnoyers
>> <mathieu.desnoyers@...icios.com> wrote:
>>>
>>> ----- On Oct 13, 2017, at 9:56 AM, Florian Weimer fweimer@...hat.com
>>> 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.

>
> Yes, that would be far more reliable.
>
> Thanks,
> Florian



-- 
Andy Lutomirski
AMA Capital Management, LLC

Powered by blists - more mailing lists