linux-kernel - Re: [PATCH] x86/xen: Fix 32-bit PV guests's usage of kernel

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140409112517.570b24e9@gandalf.local.home>
Date:	Wed, 9 Apr 2014 11:25:17 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Boris Ostrovsky <boris.ostrovsky@...cle.com>
Cc:	konrad.wilk@...cle.com, david.vrabel@...rix.com,
	xen-devel@...ts.xenproject.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86/xen: Fix 32-bit PV guests's usage of kernel_stack

On Wed, 09 Apr 2014 11:12:33 -0400
Boris Ostrovsky <boris.ostrovsky@...cle.com> wrote:

> Commit 198d208df4371734ac4728f69cb585c284d20a15 made 32-bit kernels use
> kernel_stack to point to thread_info. That change missed a couple of
> updates needed by Xen's PV guests:
> 
> 1. kernel_stack needs to be initialized for secondary CPUs
> 2. GET_THREAD_INFO() now uses %fs register which may not be the kernel's
> version when executing xen_iret()
> 
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@...cle.com>
> ---
>  arch/x86/xen/smp.c        |    3 ++-
>  arch/x86/xen/xen-asm_32.S |    4 ++++
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
> index a18eadd..7005974 100644
> --- a/arch/x86/xen/smp.c
> +++ b/arch/x86/xen/smp.c
> @@ -441,10 +441,11 @@ static int xen_cpu_up(unsigned int cpu, struct task_struct *idle)
>  	irq_ctx_init(cpu);
>  #else
>  	clear_tsk_thread_flag(idle, TIF_FORK);
> +#endif
>  	per_cpu(kernel_stack, cpu) =
>  		(unsigned long)task_stack_page(idle) -
>  		KERNEL_STACK_OFFSET + THREAD_SIZE;
> -#endif
> +
>  	xen_setup_runstate_info(cpu);
>  	xen_setup_timer(cpu);
>  	xen_init_lock_cpu(cpu);
> diff --git a/arch/x86/xen/xen-asm_32.S b/arch/x86/xen/xen-asm_32.S
> index 33ca6e4..d433637 100644
> --- a/arch/x86/xen/xen-asm_32.S
> +++ b/arch/x86/xen/xen-asm_32.S
> @@ -88,7 +88,11 @@ ENTRY(xen_iret)
>  	 * avoid having to reload %fs

Looks like this patch makes the above comment incorrect.

>  	 */
>  #ifdef CONFIG_SMP
> +	pushw %fs
> +	movl $(__KERNEL_PERCPU), %eax
> +	movl %eax, %fs
>  	GET_THREAD_INFO(%eax)
> +	popw %fs
>  	movl %ss:TI_cpu(%eax), %eax

Hmm, you're using GET_THREAD_INFO() to get the thread info to find the
current CPU for later use to avoid using %fs. ??

Is it possible to just push the cpu on the kernel irq stack and be able
to retrieve it manually? Looks like the only reason to use the
thread_info here is to get to the CPU index for below.

>  	movl %ss:__per_cpu_offset(,%eax,4), %eax
>  	mov %ss:xen_vcpu(%eax), %eax

-- Steve

--
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/