linux-kernel - Re: [PATCH] x86/entry/64: randomize kernel stack offset upon syscall

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 2 May 2019 07:47:39 -0700
From:   Andy Lutomirski <luto@...nel.org>
To:     David Laight <David.Laight@...lab.com>
Cc:     "Reshetova, Elena" <elena.reshetova@...el.com>,
        "Theodore Ts'o" <tytso@....edu>,
        Eric Biggers <ebiggers3@...il.com>,
        "ebiggers@...gle.com" <ebiggers@...gle.com>,
        "herbert@...dor.apana.org.au" <herbert@...dor.apana.org.au>,
        Ingo Molnar <mingo@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        "keescook@...omium.org" <keescook@...omium.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        "luto@...nel.org" <luto@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "jpoimboe@...hat.com" <jpoimboe@...hat.com>,
        "jannh@...gle.com" <jannh@...gle.com>,
        "Perla, Enrico" <enrico.perla@...el.com>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "bp@...en8.de" <bp@...en8.de>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
Subject: Re: [PATCH] x86/entry/64: randomize kernel stack offset upon syscall

On Thu, May 2, 2019 at 2:23 AM David Laight <David.Laight@...lab.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