public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Vegard Nossum <vegard.nossum@gmail.com>,
	Patrick McHardy <kaber@trash.net>,
	Linux Kernel Mailinglist <linux-kernel@vger.kernel.org>,
	Chuck Ebbert <cebbert@redhat.com>,
	x86@kernel.org
Subject: Re: 2.6.26-git: NULL pointer deref in __switch_to
Date: Sat, 14 Jun 2008 08:20:14 +0200	[thread overview]
Message-ID: <20080614062014.GA7340@elte.hu> (raw)
In-Reply-To: <20080613224711.GA15084@linux-os.sc.intel.com>


* Suresh Siddha <suresh.b.siddha@intel.com> wrote:

> Somehow (as described below?) TS_USEDFPU is set but the fpu is not 
> allocated or freed.
> 
> Please try the appended patch.

i've queued up your fix in tip/x86/urgent. (Git access coordinates: 
http://people.redhat.com/mingo/tip.git/README)

i'm wondering why this problem was not hit more frequently. Does it need 
some special FPU use to trigger? Or does it need an exec() with the FPU 
stack still active? (normally the FPU stack is empty at exec() time)

	Ingo

-------------------->
commit fade25af0c80b9caed83beb0b961559f4c33a49f
Author: Suresh Siddha <suresh.b.siddha@intel.com>
Date:   Fri Jun 13 15:47:12 2008 -0700

    x86: fix NULL pointer deref in __switch_to
    
    Patrick McHardy reported a crash:
    
    > > I get this oops once a day, its apparently triggered by something
    > > run by cron, but the process is a different one each time.
    > >
    > > Kernel is -git from yesterday shortly before the -rc6 release
    > > (last commit is the usb-2.6 merge, the x86 patches are missing),
    > > .config is attached.
    > >
    > > I'll retry with current -git, but the patches that have gone in
    > > since I last updated don't look related.
    > >
    > > [62060.043009] BUG: unable to handle kernel NULL pointer dereference at
    > > 000001ff
    > > [62060.043009] IP: [<c0102a9b>] __switch_to+0x2f/0x118
    > > [62060.043009] *pde = 00000000
    > > [62060.043009] Oops: 0002 [#1] PREEMPT
    
    Vegard Nossum analyzed it:
    
    > This decodes to
    >
    >    0:   0f ae 00                fxsave (%eax)
    >
    > so it's related to the floating-point context. This is the exact
    > location of the crash:
    >
    > $ addr2line -e arch/x86/kernel/process_32.o -i ab0
    > include/asm/i387.h:232
    > include/asm/i387.h:262
    > arch/x86/kernel/process_32.c:595
    >
    > ...so it looks like prev_task->thread.xstate->fxsave has become NULL.
    > Or maybe it never had any other value.
    
    Somehow (as described below) TS_USEDFPU is set but the fpu is not
    allocated or freed.
    
    Another possible FPU pre-emption issue with the sleazy FPU optimization
    which was benign before but not so anymore, with the dynamic FPU allocation
    patch.
    
    New task is getting exec'd and it is prempted at the below point.
    
    flush_thread() {
    	...
    	/*
    	* Forget coprocessor state..
    	*/
    	clear_fpu(tsk);
    		<----- Preemption point
    	clear_used_math();
    	...
    }
    
    Now when it context switches in again, as the used_math() is still set
    and fpu_counter can be > 5, we will do a math_state_restore() which sets
    the task's TS_USEDFPU. After it continues from the above preemption point
    it does clear_used_math() and much later free_thread_xstate().
    
    Now, at the next context switch, it is quite possible that xstate is
    null, used_math() is not set and TS_USEDFPU is still set. This will
    trigger unlazy_fpu() causing kernel oops.
    
    Fix this  by clearing tsk's fpu_counter before clearing task's fpu.
    
    Reported-by: Patrick McHardy <kaber@trash.net>
    Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
    Signed-off-by: Ingo Molnar <mingo@elte.hu>

diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c
index 6d54833..e2db9ac 100644
--- a/arch/x86/kernel/process_32.c
+++ b/arch/x86/kernel/process_32.c
@@ -333,6 +333,7 @@ void flush_thread(void)
 	/*
 	 * Forget coprocessor state..
 	 */
+	tsk->fpu_counter = 0;
 	clear_fpu(tsk);
 	clear_used_math();
 }
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index ac54ff5..c6eb5c9 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -294,6 +294,7 @@ void flush_thread(void)
 	/*
 	 * Forget coprocessor state..
 	 */
+	tsk->fpu_counter = 0;
 	clear_fpu(tsk);
 	clear_used_math();
 }

  reply	other threads:[~2008-06-14  6:20 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-13 17:42 2.6.26-git: NULL pointer deref in __switch_to Patrick McHardy
2008-06-13 18:24 ` Vegard Nossum
2008-06-13 22:47   ` Suresh Siddha
2008-06-14  6:20     ` Ingo Molnar [this message]
2008-06-14  7:39       ` Patrick McHardy
2008-06-16 11:06       ` Jens Axboe
2008-06-14  7:36     ` Patrick McHardy
2008-06-16 10:15     ` Simon Holm Thøgersen
2008-06-16 10:29       ` Patrick McHardy
2008-06-16 12:10         ` Patrick McHardy
2008-06-16 17:49       ` Suresh Siddha
2008-06-16 21:21         ` Simon Holm Thøgersen
2008-06-17 23:50           ` Suresh Siddha
2008-06-18  5:34             ` Rusty Russell
2008-06-18  6:23               ` Suresh Siddha
2008-06-18 12:19                 ` Rusty Russell
2008-06-18  8:42             ` Patrick McHardy
2008-06-18 13:57             ` Simon Holm Thøgersen
2008-06-13 20:10 ` Rafael J. Wysocki
2008-06-14  7:33   ` Patrick McHardy

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=20080614062014.GA7340@elte.hu \
    --to=mingo@elte.hu \
    --cc=cebbert@redhat.com \
    --cc=kaber@trash.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=suresh.b.siddha@intel.com \
    --cc=vegard.nossum@gmail.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox