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

[Ingo Molnar]

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@xxxxxxxx>
    | 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@xxxxxxx>
---
 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/