Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200907131613.12703-1-joro@8bytes.org> <20200907131613.12703-46-joro@8bytes.org>
In-Reply-To: <20200907131613.12703-46-joro@8bytes.org>
From:   Lai Jiangshan <jiangshanlai+lkml@gmail.com>
Date:   Sun, 24 Jan 2021 22:11:14 +0800
Message-ID: <CAJhGHyCMMCY9bZauzrSeQr_62SpJgZQEQy9P7Rh28HXJtF5O5A@mail.gmail.com>
Subject: Re: [PATCH v7 45/72] x86/entry/64: Add entry code for #VC handler
To:     Joerg Roedel <joro@8bytes.org>
Cc:     X86 ML <x86@kernel.org>, Joerg Roedel <jroedel@suse.de>,
        "H. Peter Anvin" <hpa@zytor.com>,
        Andy Lutomirski <luto@kernel.org>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Jiri Slaby <jslaby@suse.cz>,
        Dan Williams <dan.j.williams@intel.com>,
        Tom Lendacky <thomas.lendacky@amd.com>,
        Juergen Gross <jgross@suse.com>,
        Kees Cook <keescook@chromium.org>,
        David Rientjes <rientjes@google.com>,
        Cfir Cohen <cfir@google.com>,
        Erdem Aktas <erdemaktas@google.com>,
        Masami Hiramatsu <mhiramat@kernel.org>,
        Mike Stunes <mstunes@vmware.com>,
        Sean Christopherson <sean.j.christopherson@intel.com>,
        Martin Radev <martin.b.radev@gmail.com>,
        LKML <linux-kernel@vger.kernel.org>, kvm@vger.kernel.org,
        virtualization@lists.linux-foundation.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

> +
> +       /*
> +        * No need to switch back to the IST stack. The current stack is either
> +        * identical to the stack in the IRET frame or the VC fall-back stack,
> +        * so it is definitly mapped even with PTI enabled.
> +        */
> +       jmp     paranoid_exit
> +
>

Hello

I know we don't enable PTI on AMD, but the above comment doesn't align to the
next code.

We assume PTI is enabled as the comments said "even with PTI enabled".

When #VC happens after entry_SYSCALL_64 but before it switches to the
kernel CR3.  vc_switch_off_ist() will switch the stack to the kernel stack
and paranoid_exit can't work when it switches to user CR3 on the kernel stack.

The comment above lost information that the current stack is possible to be
the kernel stack which is mapped not user CR3.

Maybe I missed something.

Thanks
Lai

> +#ifdef CONFIG_AMD_MEM_ENCRYPT
> +asmlinkage __visible noinstr struct pt_regs *vc_switch_off_ist(struct pt_regs *regs)
> +{
> +       unsigned long sp, *stack;
> +       struct stack_info info;
> +       struct pt_regs *regs_ret;
> +
> +       /*
> +        * In the SYSCALL entry path the RSP value comes from user-space - don't
> +        * trust it and switch to the current kernel stack
> +        */
> +       if (regs->ip >= (unsigned long)entry_SYSCALL_64 &&
> +           regs->ip <  (unsigned long)entry_SYSCALL_64_safe_stack) {
> +               sp = this_cpu_read(cpu_current_top_of_stack);
> +               goto sync;
> +       }