linux-kernel - Re: PATCH? process_32.c:__switch_to() calls __math_state_restore() before updating current

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: <1265305253.2768.7.camel@sbs-t61.sc.intel.com>
Date:	Thu, 04 Feb 2010 09:40:53 -0800
From:	Suresh Siddha <suresh.b.siddha@...el.com>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	Arjan van de Ven <arjan@...ux.intel.com>,
	Jeremy Fitzhardinge <jeremy.fitzhardinge@...rix.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: PATCH? process_32.c:__switch_to() calls __math_state_restore()
 before updating current_task

On Thu, 2010-02-04 at 08:51 -0800, Oleg Nesterov wrote:
> I didn't try to verify __switch_to()->__math_state_restore() is really
> wrong, this is more the question than the patch. But at least the code
> looks wrong, it calls __math_state_restore() which uses curent before
> current_task was updated.
> 
> Uncompiled/untested.
> 
> Signed-off-by: Oleg Nesterov <oleg@...hat.com>
> 
> --- a/arch/x86/kernel/process_32.c
> +++ b/arch/x86/kernel/process_32.c
> @@ -377,9 +377,6 @@ __switch_to(struct task_struct *prev_p, 
>  	 */
>  	arch_end_context_switch(next_p);
>  
> -	if (preload_fpu)
> -		__math_state_restore();
> -
>  	/*
>  	 * Restore %gs if needed (which is common)
>  	 */
> @@ -388,6 +385,9 @@ __switch_to(struct task_struct *prev_p, 
>  
>  	percpu_write(current_task, next_p);
>  
> +	if (preload_fpu)
> +		__math_state_restore();
> +
>  	return prev_p;
>  }

Oleg, __math_state_restore() uses current_thread_info() which at that
point already has the right esp and as such uses the correct thread
struct etc.

After saying that, in the past I have also ran into this question and
got satisfied by looking deeper. Best is to make the 32bit and 64bit
code similar as much as possible and as such your patch is acceptable.

Can you please re-post with a proper changelog (and ofcourse testing
etc)? You can add my Ack to that.

thanks,
suresh

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