linux-kernel - [patch] x86: fix savesegment() bug causing crashes on 64-bit

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20080711175002.GA32116@elte.hu>
Date:	Fri, 11 Jul 2008 19:50:02 +0200
From:	Ingo Molnar <mingo@...e.hu>
To:	Jeremy Fitzhardinge <jeremy@...p.org>
Cc:	linux-kernel@...r.kernel.org,
	the arch/x86 maintainers <x86@...nel.org>,
	Arjan van de Ven <arjan@...radead.org>
Subject: [patch] x86: fix savesegment() bug causing crashes on 64-bit

    
i spent a fair amount of time chasing a 64-bit bootup crash in 
tip/master that manifested itself as bootup segfaults:
    
      S10network[1825]: segfault at 7f3e2b5d16b8 ip 00000031108748c9 sp 00007fffb9c14c70 error 4 in libc-2.7.so[3110800000+14d000]
    
eventually causing init to die and panic the system:
    
      Kernel panic - not syncing: Attempted to kill init!
      Pid: 1, comm: init Not tainted 2.6.26-rc9-tip #13878
    
after a marathonic bisection session, the bad commit turned out to be 
this one in tip/x86/*:
    
    | b7675791859075418199c7af86a116ea34eaf5bd is first bad commit
    | commit b7675791859075418199c7af86a116ea34eaf5bd
    | Author: Jeremy Fitzhardinge <jeremy@...p.org>
    | Date:   Wed Jun 25 00:19:00 2008 -0400
    |
    |     x86: remove open-coded save/load segment operations
    |
    |     This removes a pile of buggy open-coded implementations of savesegment
    |     and loadsegment.
    
after some more bisection of this patch itself, it turns out that what
makes the difference are the savesegment() changes to __switch_to().
    
Taking a look at this portion of arch/x86/kernel/process_64.o revealed
this crutial difference:
    
    | good:    99c:       8c e0                   mov    %fs,%eax
    |          99e:       89 45 cc                mov    %eax,-0x34(%rbp)
    |
    | bad:     99c:       8c 65 cc                mov    %fs,-0x34(%rbp)
    
which is due to:
    
    |                 unsigned fsindex;
    | -               asm volatile("movl %%fs,%0" : "=r" (fsindex));
    | +               savesegment(fs, fsindex);
    
savesegment() is implemented as:
    
     #define savesegment(seg, value)                                \
              asm("mov %%" #seg ",%0":"=rm" (value) : : "memory")
    
note the "m" modifier - it allows GCC to generate the segment move
into a memory operand as well.
    
But regarding segment operands there's a subtle detail in the x86
instruction set: the above 16-bit moves are zero-extend, but only
    
If it goes to a memory operand, -0x34(%rbp) in the above case, there's
no zero-extend to 32-bit and the instruction will only save 16 bits
    
The other 16 bits is random data - which can cause problems when that 
value is used later on.
    
The solution is to only allow segment operands to go to registers. This 
fix allows my test-system to boot up without crashing.
    
Signed-off-by: Ingo Molnar <mingo@...e.hu>
---
 include/asm-x86/system.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/asm-x86/system.h b/include/asm-x86/system.h
index 45641bc..929345a 100644
--- a/include/asm-x86/system.h
+++ b/include/asm-x86/system.h
@@ -164,7 +164,7 @@ extern void native_load_gs_index(unsigned);
  * Save a segment register away
  */
 #define savesegment(seg, value)				\
-	asm("mov %%" #seg ",%0":"=rm" (value) : : "memory")
+	asm("mov %%" #seg ",%0":"=r" (value) : : "memory")
 
 static inline unsigned long get_limit(unsigned long segment)
 {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/