lkml.org 
[lkml]   [2019]   [May]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH] x86/entry/64: randomize kernel stack offset upon syscall
    On Thu, May 2, 2019 at 2:23 AM David Laight <David.Laight@aculab.com> wrote:
    >
    > From: Reshetova, Elena
    > > Sent: 02 May 2019 09:16
    > ...
    > > > I'm also guessing that get_cpu_var() disables pre-emption?
    > >
    > > Yes, in my understanding:
    > >
    > > #define get_cpu_var(var) \
    > > (*({ \
    > > preempt_disable(); \
    > > this_cpu_ptr(&var); \
    > > }))
    > >
    > > > This code could probably run 'fast and loose' and just ignore
    > > > the fact that pre-emption would have odd effects.
    > > > All it would do is perturb the randomness!
    > >
    > > Hm.. I see your point, but I am wondering what the odd effects might
    > > be.. i.e. can we end up using the same random bits twice for two or more
    > > different syscalls and attackers can try to trigger this situation?
    >
    > To trigger it you'd need to arrange for an interrupt in the right
    > timing window to cause another process to run.
    > There are almost certainly easier ways to break things.
    >
    > I think the main effects would be the increment writing to a different
    > cpu local data (causing the same data to be used again and/or skipped)
    > and the potential for updating the random buffer on the 'wrong cpu'.
    >
    > So something like:
    > /* We don't really care if the update is written to the 'wrong'
    > * cpu or if the vale comes from the wrong buffer. */
    > offset = *this_cpu_ptr(&cpu_syscall_rand_offset);
    > *this_cpu_ptr(&cpu_syscall_rand_offset) = offset + 1;
    >
    > if ((offset &= 4095)) return this_cpu_ptr(&cpu_syscall_rand_buffer)[offset];
    >
    > buffer = get_cpu_var((&cpu_syscall_rand_buffer);
    > get_random_bytes();
    > val = buffer[0];
    > /* maybe set cpu_syscall_rand_offset to 1 */
    > put_cpu_var();
    > return val;
    >
    > The whole thing might even work with a global buffer!
    >

    I don't see how this makes sense in the context of the actual entry
    code. The code looks like this right now:

    enter_from_user_mode();
    <--- IRQs off here
    local_irq_enable();

    Presumably this could become:

    enter_from_user_mode();
    if (the percpu buffer has enough bytes) {
    use them;
    local_irq_enable();
    } else {
    local_irq_enable();
    get more bytes;
    if (get_cpu() == the old cpu) {
    refill the buffer;
    } else {
    feel rather silly;
    }
    put_cpu();
    }

    everything after the enter_from_user_mode() could get renamed
    get_randstack_offset_and_irq_enable().

    Or we decide that calling get_random_bytes() is okay with IRQs off and
    this all gets a bit simpler.

    --Andy

    \
     
     \ /
      Last update: 2019-05-02 16:49    [W:2.654 / U:0.044 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site