Re: [PATCH] x86: Load __USER_DS into DS/ES after resume

[Pavel Machek <pavel@ucw.cz>]

On Sun 2015-06-14 08:56:35, Ingo Molnar wrote:
> 
> * Brian Gerst <brgerst@gmail.com> wrote:
> 
> > >> So if wakeup_pmode_return is really the first thing called then the whole 
> > >> premise of shadow descriptor corruption goes out the window: we reload all 
> > >> relevant segment registers.
> > >
> > > True, but it still leaves the fact that we're loading __KERNEL_DS instead of 
> > > __USER_DS, right?  So we end up in the kernel in some context (I have no clue 
> > > what context) with __KERNEL_DS loaded.  It's very easy for us to inadvertently 
> > > fix it: we could return to userspace by any means whatsoever except SYSEXIT, 
> > > or we could even return back to some preempted kernel context.
> > >
> > > I still think we should replace __KERNEL_DS with __USER_DS in 
> > > wakeup_pmode_return and see if the problem goes away.
> > 
> > I'm pretty sure that's what the problem is.  If you look at the sysexit path, it 
> > never reloads ds/es.  It assumes they are still __USER_DS set at sysenter.  The 
> > iret path does restore all the user segments.
> 
> Ok, so something like the patch below, right?
> 
> Thanks,
> 
> 	Ingo
> 
> =====================>
>  arch/x86/kernel/acpi/wakeup_32.S | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/acpi/wakeup_32.S b/arch/x86/kernel/acpi/wakeup_32.S
> index 665c6b7d2ea9..7302bbaea184 100644
> --- a/arch/x86/kernel/acpi/wakeup_32.S
> +++ b/arch/x86/kernel/acpi/wakeup_32.S
> @@ -12,11 +12,13 @@ ENTRY(wakeup_pmode_return)
>  wakeup_pmode_return:
>  	movw	$__KERNEL_DS, %ax
>  	movw	%ax, %ss
> -	movw	%ax, %ds
> -	movw	%ax, %es
>  	movw	%ax, %fs
>  	movw	%ax, %gs
>  
> +	movw	$__KERNEL_DS, %ax
> +	movw	%ax, %ds
> +	movw	%ax, %es

Umm. Are you sure? :-).
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html