linux-kernel - Re: [patch V3 64/66] x86/fpu: Return proper error codes from user access functions

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: <YNLxKB9a6XQHW+w3@zn.tnic>
Date:   Wed, 23 Jun 2021 10:30:32 +0200
From:   Borislav Petkov <bp@...e.de>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Andy Lutomirski <luto@...nel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Fenghua Yu <fenghua.yu@...el.com>,
        Tony Luck <tony.luck@...el.com>,
        Yu-cheng Yu <yu-cheng.yu@...el.com>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Kan Liang <kan.liang@...ux.intel.com>
Subject: Re: [patch V3 64/66] x86/fpu: Return proper error codes from user
 access functions

On Fri, Jun 18, 2021 at 04:19:27PM +0200, Thomas Gleixner wrote:
> When *RSTOR from user memory raises an exception there is no way to
> differentiate them. That's bad because it forces the slow path even when
> the failure was not a fault. If the operation raised eg. #GP then going
> through the slow path is pointless.
> 
> Use _ASM_EXTABLE_FAULT() which stores the trap number and let the exception
> fixup return the negated trap number as error.
> 
> This allows to seperate the fast path and let it handle faults directly and

separate

> avoid the slow path for all other exceptions.
> 
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> ---
>  arch/x86/include/asm/fpu/internal.h |    9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> --- a/arch/x86/include/asm/fpu/internal.h
> +++ b/arch/x86/include/asm/fpu/internal.h
> @@ -87,6 +87,7 @@ extern void fpstate_init_soft(struct swr
>  static inline void fpstate_init_soft(struct swregs_state *soft) {}
>  #endif
>  
> +/* Returns 0 or the negated trap number, which results in -EFAULT for #PF */
>  #define user_insn(insn, output, input...)				\
>  ({									\
>  	int err;							\
> @@ -94,14 +95,14 @@ static inline void fpstate_init_soft(str
>  	might_fault();							\
>  									\
>  	asm volatile(ASM_STAC "\n"					\
> -		     "1:" #insn "\n\t"					\
> +		     "1: " #insn "\n"					\
>  		     "2: " ASM_CLAC "\n"				\
>  		     ".section .fixup,\"ax\"\n"				\
> -		     "3:  movl $-1,%[err]\n"				\
> +		     "3:  negl %%eax\n"					\
>  		     "    jmp  2b\n"					\
>  		     ".previous\n"					\
> -		     _ASM_EXTABLE(1b, 3b)				\
> -		     : [err] "=r" (err), output				\
> +		     _ASM_EXTABLE_FAULT(1b, 3b)				\
> +		     : [err] "=a" (err), output				\
>  		     : "0"(0), input);					\
>  	err;								\

Don't we wanna do the same for XSTATE_OP() too?

Because restore_hwregs_from_user() could call
xrstor_from_user_sigframe() too which ends up doing XRSTOR and latter
can cause a #PF too.

Hmm.

-- 
Regards/Gruss,
    Boris.

SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg