All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Jeremy Fitzhardinge <jeremy@goop.org>
Cc: linux-kernel@vger.kernel.org,
	the arch/x86 maintainers <x86@kernel.org>,
	Arjan van de Ven <arjan@infradead.org>
Subject: [patch] x86: fix savesegment() bug causing crashes on 64-bit
Date: Fri, 11 Jul 2008 19:50:02 +0200	[thread overview]
Message-ID: <20080711175002.GA32116@elte.hu> (raw)

    
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@goop.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@elte.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)
 {

             reply	other threads:[~2008-07-11 17:50 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-11 17:50 Ingo Molnar [this message]
2008-07-11 17:58 ` [patch] x86: fix savesegment() bug causing crashes on 64-bit Jeremy Fitzhardinge

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080711175002.GA32116@elte.hu \
    --to=mingo@elte.hu \
    --cc=arjan@infradead.org \
    --cc=jeremy@goop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.