Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230327075838.5403-1-xin3.li@intel.com> <20230327075838.5403-28-xin3.li@intel.com>
In-Reply-To: <20230327075838.5403-28-xin3.li@intel.com>
From:   Lai Jiangshan <jiangshanlai@gmail.com>
Date:   Fri, 31 Mar 2023 16:03:24 +0800
Message-ID: <CAJhGHyDQSHkfxVS1o+rqA4JaaOpvyVH0C5JNLKdDuzNtncQiTw@mail.gmail.com>
Subject: Re: [PATCH v6 27/33] x86/fred: fixup fault on ERETU by jumping to fred_entrypoint_user
To:     Xin Li <xin3.li@intel.com>
Cc:     linux-kernel@vger.kernel.org, x86@kernel.org, kvm@vger.kernel.org,
        tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
        dave.hansen@linux.intel.com, hpa@zytor.com, peterz@infradead.org,
        andrew.cooper3@citrix.com, seanjc@google.com, pbonzini@redhat.com,
        ravi.v.shankar@intel.com, shan.kang@intel.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Mar 27, 2023 at 4:24=E2=80=AFPM Xin Li <xin3.li@intel.com> wrote:
>
> If the stack frame contains an invalid user context (e.g. due to invalid =
SS,
> a non-canonical RIP, etc.) the ERETU instruction will trap (#SS or #GP).
>
> From a Linux point of view, this really should be considered a user space
> failure, so use the standard fault fixup mechanism to intercept the fault=
,
> fix up the exception frame, and redirect execution to fred_entrypoint_use=
r.
> The end result is that it appears just as if the hardware had taken the
> exception immediately after completing the transition to user space.
>
> Suggested-by: H. Peter Anvin (Intel) <hpa@zytor.com>
> Tested-by: Shan Kang <shan.kang@intel.com>
> Signed-off-by: Xin Li <xin3.li@intel.com>
> ---
>
> Changes since v5:
> * Move the NMI bit from an invalid stack frame, which caused ERETU to fau=
lt,
>   to the fault handler's stack frame, thus to unblock NMI ASAP if NMI is =
blocked
>   (Lai Jiangshan).
> ---
>  arch/x86/entry/entry_64_fred.S             |  8 +++--
>  arch/x86/include/asm/extable_fixup_types.h |  4 ++-
>  arch/x86/mm/extable.c                      | 36 ++++++++++++++++++++++
>  3 files changed, 45 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/entry/entry_64_fred.S b/arch/x86/entry/entry_64_fre=
d.S
> index d975cacd060f..efe2bcd11273 100644
> --- a/arch/x86/entry/entry_64_fred.S
> +++ b/arch/x86/entry/entry_64_fred.S
> @@ -5,8 +5,10 @@
>   * The actual FRED entry points.
>   */
>  #include <linux/linkage.h>
> -#include <asm/errno.h>
> +#include <asm/asm.h>
>  #include <asm/asm-offsets.h>
> +#include <asm/errno.h>
> +#include <asm/export.h>
>  #include <asm/fred.h>
>
>  #include "calling.h"
> @@ -38,7 +40,9 @@ SYM_CODE_START_NOALIGN(fred_entrypoint_user)
>         call    fred_entry_from_user
>  SYM_INNER_LABEL(fred_exit_user, SYM_L_GLOBAL)
>         FRED_EXIT
> -       ERETU
> +1:     ERETU
> +
> +       _ASM_EXTABLE_TYPE(1b, fred_entrypoint_user, EX_TYPE_ERETU)
>  SYM_CODE_END(fred_entrypoint_user)
>
>  .fill fred_entrypoint_kernel - ., 1, 0xcc
> diff --git a/arch/x86/include/asm/extable_fixup_types.h b/arch/x86/includ=
e/asm/extable_fixup_types.h
> index 991e31cfde94..1585c798a02f 100644
> --- a/arch/x86/include/asm/extable_fixup_types.h
> +++ b/arch/x86/include/asm/extable_fixup_types.h
> @@ -64,6 +64,8 @@
>  #define        EX_TYPE_UCOPY_LEN4              (EX_TYPE_UCOPY_LEN | EX_D=
ATA_IMM(4))
>  #define        EX_TYPE_UCOPY_LEN8              (EX_TYPE_UCOPY_LEN | EX_D=
ATA_IMM(8))
>
> -#define EX_TYPE_ZEROPAD                        20 /* longword load with =
zeropad on fault */
> +#define        EX_TYPE_ZEROPAD                 20 /* longword load with =
zeropad on fault */
> +
> +#define        EX_TYPE_ERETU                   21
>
>  #endif
> diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
> index 60814e110a54..a5d75b27a993 100644
> --- a/arch/x86/mm/extable.c
> +++ b/arch/x86/mm/extable.c
> @@ -6,6 +6,7 @@
>  #include <xen/xen.h>
>
>  #include <asm/fpu/api.h>
> +#include <asm/fred.h>
>  #include <asm/sev.h>
>  #include <asm/traps.h>
>  #include <asm/kdebug.h>
> @@ -195,6 +196,37 @@ static bool ex_handler_ucopy_len(const struct except=
ion_table_entry *fixup,
>         return ex_handler_uaccess(fixup, regs, trapnr);
>  }
>
> +#ifdef CONFIG_X86_FRED
> +static bool ex_handler_eretu(const struct exception_table_entry *fixup,
> +                            struct pt_regs *regs, unsigned long error_co=
de)
> +{
> +       struct pt_regs *uregs =3D (struct pt_regs *)(regs->sp - offsetof(=
struct pt_regs, ip));
> +       unsigned short ss =3D uregs->ss;
> +       unsigned short cs =3D uregs->cs;
> +
> +       /*
> +        * Move the NMI bit from the invalid stack frame, which caused ER=
ETU
> +        * to fault, to the fault handler's stack frame, thus to unblock =
NMI
> +        * with the fault handler's ERETS instruction ASAP if NMI is bloc=
ked.
> +        */
> +       regs->nmi =3D uregs->nmi;
> +
> +       fred_info(uregs)->edata =3D fred_event_data(regs);
> +       uregs->ssx =3D regs->ssx;
> +       uregs->ss =3D ss;
> +       uregs->csx =3D regs->csx;
> +       uregs->nmi =3D 0; /* The NMI bit was moved away above */
> +       uregs->current_stack_level =3D 0;
> +       uregs->cs =3D cs;
> +
> +       /* Copy error code to uregs and adjust stack pointer accordingly =
*/
> +       uregs->orig_ax =3D error_code;

The address of uregs->orig_ax is below regs->sp, so I think
some comments are needed here to state why it is safe to
write to uregs->orig_ax (a.k.a it is not verlapped with regs).


Thanks
Lai

> +       regs->sp -=3D 8;
> +
> +       return ex_handler_default(fixup, regs);
> +}
> +#endif
> +
>  int ex_get_fixup_type(unsigned long ip)
>  {
>         const struct exception_table_entry *e =3D search_exception_tables=
(ip);
> @@ -272,6 +304,10 @@ int fixup_exception(struct pt_regs *regs, int trapnr=
, unsigned long error_code,
>                 return ex_handler_ucopy_len(e, regs, trapnr, reg, imm);
>         case EX_TYPE_ZEROPAD:
>                 return ex_handler_zeropad(e, regs, fault_addr);
> +#ifdef CONFIG_X86_FRED
> +       case EX_TYPE_ERETU:
> +               return ex_handler_eretu(e, regs, error_code);
> +#endif
>         }
>         BUG();
>  }
> --
> 2.34.1
>