Re: [RFC PATCH 1/5] x86: add user_mode_vm check in stack_overflow_check

[HAYASAKA Mitsuo <mitsuo.hayasaka.hu@hitachi.com>]

(2011/11/11 4:52), Konrad Rzeszutek Wilk wrote:
> On Mon, Nov 07, 2011 at 02:52:35PM +0900, Mitsuo Hayasaka wrote:
>> The kernel stack overflow is checked in stack_overflow_check(),
>> which may wrongly detect the overflow if the stack pointer
>> pointed to the kernel stack accidentally.
> 
> I think you mean to say 'points'.

Yes. Thank you for your correction.

> 
> How do we accidently point the stack pointer to the kernel stack?

I guess it may happen due to a kind of stack overflow, although
I've not succeeded to `accidentally' point the stack pointer to
the kernel stack yet...

At least, we can intentionally cause the overflow message, using the
following program in user space although the overflow does not occur.

=============================================
#include <stdio.h>
#include <sys/types.h>
#include <unistd.h>

int main(int argc, char *argv[]) {
	unsigned long long dummyRSP;

	printf("PID:%d\n", getpid());
	printf("Input dummyRSP address: ");
	scanf("%Lx", &dummyRSP);
	printf("DummyRSP address is %Lx\n", dummyRSP);
	puts("Replace RSP with dummyRSP...");
	__asm__ ("movq %0,%%rsp": : "r" (dummyRSP));
	while(1) ;
}
=============================================

We need to give this program a dummy RSP address that must point to
an overflow address in kernel stack and can be gotten using tools such
as crash. These steps are summarized as follows.

(1) Execute this program and get the pid.
(2) Execute crash and put "task <pid>"
(3) Get the address indicated by stack field of task_struct
(4) Input the address to this program

The big problem is that user space program can directly control the
stack overflow checking in kernel space, regardless of intentional or
accidental operation. In other words, the kernel stack overflow is
never detected after execution of this program.

So, I would like to fix this problem.

> 
>>
>> This patch adds user-mode-vm checking before it to avoid this
>> misdetection and bails out early if the user stack is used.
>>
>> Signed-off-by: Mitsuo Hayasaka <mitsuo.hayasaka.hu@hitachi.com>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: "H. Peter Anvin" <hpa@zytor.com>
>> ---
>>
>>  arch/x86/kernel/irq_64.c |    3 +++
>>  1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/x86/kernel/irq_64.c b/arch/x86/kernel/irq_64.c
>> index acf8fbf..69bca46 100644
>> --- a/arch/x86/kernel/irq_64.c
>> +++ b/arch/x86/kernel/irq_64.c
>> @@ -38,6 +38,9 @@ static inline void stack_overflow_check(struct pt_regs *regs)
>>  #ifdef CONFIG_DEBUG_STACKOVERFLOW
>>  	u64 curbase = (u64)task_stack_page(current);
>>  
>> +	if (user_mode_vm(regs))
>> +		return;
>> +
>>  	WARN_ONCE(regs->sp >= curbase &&
>>  		  regs->sp <= curbase + THREAD_SIZE &&
>>  		  regs->sp <  curbase + sizeof(struct thread_info) +
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/
>