Eric Lee / smarc-ti-linux-kernel

Download as
Browse Code ยป

Commit 7b6b3d67f0d7d67426da74611698a74ffb33bea7

Authored by Andy Lutomirski
Committed by Greg Kroah-Hartman
1 parent e3b3b6d580

x86, tls: Interpret an all-zero struct user_desc as "no segment" 

commit 3669ef9fa7d35f573ec9c0e0341b29251c2734a7 upstream.

The Witcher 2 did something like this to allocate a TLS segment index:

        struct user_desc u_info;
        bzero(&u_info, sizeof(u_info));
        u_info.entry_number = (uint32_t)-1;

        syscall(SYS_set_thread_area, &u_info);

Strictly speaking, this code was never correct.  It should have set
read_exec_only and seg_not_present to 1 to indicate that it wanted
to find a free slot without putting anything there, or it should
have put something sensible in the TLS slot if it wanted to allocate
a TLS entry for real.  The actual effect of this code was to
allocate a bogus segment that could be used to exploit espfix.

The set_thread_area hardening patches changed the behavior, causing
set_thread_area to return -EINVAL and crashing the game.

This changes set_thread_area to interpret this as a request to find
a free slot and to leave it empty, which isn't *quite* what the game
expects but should be close enough to keep it working.  In
particular, using the code above to allocate two segments will
allocate the same segment both times.

According to FrostbittenKing on Github, this fixes The Witcher 2.

If this somehow still causes problems, we could instead allocate
a limit==0 32-bit data segment, but that seems rather ugly to me.

Fixes: 41bdc78544b8 x86/tls: Validate TLS entries to protect espfix
Signed-off-by: Andy Lutomirski <luto@amacapital.net>
Cc: torvalds@linux-foundation.org
Link: http://lkml.kernel.org/r/0cb251abe1ff0958b8e468a9a9a905b80ae3a746.1421954363.git.luto@amacapital.net
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Showing 2 changed files with 36 additions and 2 deletions Side-by-side Diff

arch/x86/include/asm/desc.h
Diff comments   View file @ 7b6b3d6
... ... @@ -262,6 +262,19 @@
262 262 (info)->seg_not_present == 1 && \
263 263 (info)->useable == 0)
264 264  
  265 +/* Lots of programs expect an all-zero user_desc to mean "no segment at all". */
  266 +static inline bool LDT_zero(const struct user_desc *info)
  267 +{
  268 + return (info->base_addr == 0 &&
  269 + info->limit == 0 &&
  270 + info->contents == 0 &&
  271 + info->read_exec_only == 0 &&
  272 + info->seg_32bit == 0 &&
  273 + info->limit_in_pages == 0 &&
  274 + info->seg_not_present == 0 &&
  275 + info->useable == 0);
  276 +}
  277 +
265 278 static inline void clear_LDT(void)
266 279 {
267 280 set_ldt(NULL, 0);
arch/x86/kernel/tls.c
Diff comments   View file @ 7b6b3d6
... ... @@ -29,7 +29,28 @@
29 29  
30 30 static bool tls_desc_okay(const struct user_desc *info)
31 31 {
32   - if (LDT_empty(info))
  32 + /*
  33 + * For historical reasons (i.e. no one ever documented how any
  34 + * of the segmentation APIs work), user programs can and do
  35 + * assume that a struct user_desc that's all zeros except for
  36 + * entry_number means "no segment at all". This never actually
  37 + * worked. In fact, up to Linux 3.19, a struct user_desc like
  38 + * this would create a 16-bit read-write segment with base and
  39 + * limit both equal to zero.
  40 + *
  41 + * That was close enough to "no segment at all" until we
  42 + * hardened this function to disallow 16-bit TLS segments. Fix
  43 + * it up by interpreting these zeroed segments the way that they
  44 + * were almost certainly intended to be interpreted.
  45 + *
  46 + * The correct way to ask for "no segment at all" is to specify
  47 + * a user_desc that satisfies LDT_empty. To keep everything
  48 + * working, we accept both.
  49 + *
  50 + * Note that there's a similar kludge in modify_ldt -- look at
  51 + * the distinction between modes 1 and 0x11.
  52 + */
  53 + if (LDT_empty(info) || LDT_zero(info))
33 54 return true;
34 55  
35 56 /*
... ... @@ -71,7 +92,7 @@
71 92 cpu = get_cpu();
72 93  
73 94 while (n-- > 0) {
74   - if (LDT_empty(info))
  95 + if (LDT_empty(info) || LDT_zero(info))
75 96 desc->a = desc->b = 0;
76 97 else
77 98 fill_ldt(desc, info);