linux-kernel - Re: [PATCH v3 05/10] x86/fpu/xstate: Define new functions for clearing fpregs and xstates

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]

Date:   Wed, 29 Apr 2020 18:39:06 +0200
From:   Borislav Petkov <bp@...en8.de>
To:     Yu-cheng Yu <yu-cheng.yu@...el.com>
Cc:     linux-kernel@...r.kernel.org, x86@...nel.org,
        "H. Peter Anvin" <hpa@...or.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Tony Luck <tony.luck@...el.com>,
        Andy Lutomirski <luto@...nel.org>,
        Rik van Riel <riel@...riel.com>,
        "Ravi V. Shankar" <ravi.v.shankar@...el.com>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Fenghua Yu <fenghua.yu@...el.com>,
        Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH v3 05/10] x86/fpu/xstate: Define new functions for
 clearing fpregs and xstates

On Wed, Apr 29, 2020 at 09:06:44AM -0700, Yu-cheng Yu wrote:
> From: Fenghua Yu <fenghua.yu@...el.com>
> 
> Currently, fpu__clear() clears all fpregs and xstates.  Once XSAVES
> supervisor states are introduced, supervisor settings (e.g. CET xstates)
> must remain active for signals; It is necessary to have separate functions:
> 
> - Create fpu__clear_user_states(): clear only user settings for signals;
> - Create fpu__clear_all(): clear both user and supervisor settings in
>    flush_thread().
> 
> Also modify copy_init_fpstate_to_fpregs() to take a mask from above two
> functions.
> 
> Signed-off-by: Fenghua Yu <fenghua.yu@...el.com>
> Co-developed-by: Yu-cheng Yu <yu-cheng.yu@...el.com>
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@...el.com>
> Reviewed-by: Dave Hansen <dave.hansen@...ux.intel.com>
> Reviewed-by: Tony Luck <tony.luck@...el.com>
> 
> v3:
> - Put common code into a static function fpu__clear(), with a parameter
>   user_only.
> 
> v2:
> - Fixed an issue where fpu__clear_user_states() drops supervisor xstates.
> - Revise commit log.

Try applying that patch from this mail yourself and see whether the
patch changelog will remain in the commit message or it will get
discarded.

> @@ -318,18 +313,40 @@ static inline void copy_init_fpstate_to_fpregs(void)
>   * Called by sys_execve(), by the signal handler code and by various
>   * error paths.
>   */
> -void fpu__clear(struct fpu *fpu)
> +static void fpu__clear(struct fpu *fpu, int user_only)
>  {
> -	WARN_ON_FPU(fpu != &current->thread.fpu); /* Almost certainly an anomaly */
> +	WARN_ON_FPU(fpu != &current->thread.fpu);

Why did you remove the side comment?

Is it wrong?

Why do you do such arbitrary changes which are not needed instead of
concentrating on only the changes the patch should do?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette