Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 26 Jun 2018 21:33:06 +0200
From: Solar Designer <solar@...nwall.com>
To: owl-dev@...ts.openwall.com
Cc: Vasily Averin <vvs@...tuozzo.com>
Subject: Re: 32-bit syscall breakage in -431 kernel with KAISER

On Tue, Jun 26, 2018 at 09:13:43PM +0200, Solar Designer wrote:
> Reviewing the diffs between e.g. -423 and -431, I see this stack[] in
> struct tss is in fact newly added:
> 
> @@ -245,7 +248,29 @@
>          * 8 bytes, for an extra "long" of ~0UL
>          */
>         unsigned long io_bitmap[IO_BITMAP_LONGS + 1];
> -} __attribute__((packed)) ____cacheline_aligned;
> +       /*
> +        *
> +        * The Intel SDM says (Volume 3, 7.2.1):
> +        *
> +        *  Avoid placing a page boundary in the part of the TSS that the
> +        *  processor reads during a task switch (the first 104 bytes). The
> +        *  processor may not correctly perform address translations if a
> +        *  boundary occurs in this area. During a task switch, the processor
> +        *  reads and writes into the first 104 bytes of each TSS (using
> +        *  contiguous physical addresses beginning with the physical address
> +        *  of the first byte of the TSS). So, after TSS access begins, if
> +        *  part of the 104 bytes is not physically contiguous, the processor
> +        *  will access incorrect information without generating a page-fault
> +        *  exception.
> +        *
> +        * There are also a lot of errata involving the TSS spanning a page
> +        * boundary.  Assert that we're not doing that.
> +        */
> +#ifndef __GENKSYMS__
> +       unsigned long           stack_canary;
> +       unsigned long           stack[64];
> +#endif
> +} __attribute__((packed, __aligned__(PAGE_SIZE)));

BTW, it'd be fun if this tiny stack ever overflows.  The canary check
doesn't do much:

@@ -39,6 +41,9 @@ static inline void stack_overflow_check(
        u64 curbase = (u64) current->thread_info;
        static unsigned long warned = -60*HZ;
 
+       if (WARN_ON(__get_cpu_var(init_tss.stack_canary) != STACK_END_MAGIC))
+               __get_cpu_var(init_tss.stack_canary) = STACK_END_MAGIC;

Looks like the attacker would be able to access some high-numbered I/O
ports from userspace then.

I guess the assumption here is that stack overflow can only happen as a
result of changes to the kernel, and would presumably be detected due to
the warning before that kernel revision is shipped.

Alexander

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.