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: <20220412121541.4595-1-jiangshanlai@gmail.com> <20220412121541.4595-8-jiangshanlai@gmail.com>
 <877d7d31fq.ffs@tglx>
In-Reply-To: <877d7d31fq.ffs@tglx>
From:   Lai Jiangshan <jiangshanlai@gmail.com>
Date:   Mon, 25 Apr 2022 21:25:18 +0800
Message-ID: <CAJhGHyAuu6RN-uFsu8XocFLewX1M=c7e+K948meobndXV5PLYQ@mail.gmail.com>
Subject: Re: [PATCH V5 7/7] x86/entry: Use idtentry macro for entry_INT80_compat
To:     Thomas Gleixner <tglx@linutronix.de>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        Borislav Petkov <bp@alien8.de>,
        Peter Zijlstra <peterz@infradead.org>,
        Josh Poimboeuf <jpoimboe@redhat.com>,
        Andy Lutomirski <luto@kernel.org>, X86 ML <x86@kernel.org>,
        Lai Jiangshan <jiangshan.ljs@antgroup.com>,
        Ingo Molnar <mingo@redhat.com>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        "H. Peter Anvin" <hpa@zytor.com>, Joerg Roedel <jroedel@suse.de>,
        "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
        "Chang S. Bae" <chang.seok.bae@intel.com>,
        Kees Cook <keescook@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Apr 25, 2022 at 6:24 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Tue, Apr 12 2022 at 20:15, Lai Jiangshan wrote:
> > From: Lai Jiangshan <jiangshan.ljs@antgroup.com>
> >
> > entry_INT80_compat is identical to idtentry macro except a special
> > handling for %rax in the prolog.
>
> Seriously?
>
> > -     pushq   %rsi                    /* pt_regs->si */
> > -     xorl    %esi, %esi              /* nospec   si */
>
> esi is not cleared in CLEAR_REGS. So much for identical.
>
>

Hello, Thomas

Thank you for the review.

They (the old entry_INT80_compat() and the new using the macro
idtentry) are not identical in ASM code.

The macro idtentry pushes %rsi to the entry stack and then copies
it from the entry stack to the kernel stack and then switches
the stack.

The original entry_INT80_compat() is much more straightforward
and efficient.  It switches the stack as soon as possible and
then pushes %rsi directly onto the kernel stack.

So they are different in this aspect.

I compared the macro idtentry and the original entry_INT80_compat(),
to check if the macro idtentry has all the behaviors that the INT80
thing has and check if what the macro idtentry has but the INT80
thing doesn't is a No-op (like the handling of bad IRET).

In my view, the checks don't fail my expectations except for
regs->ax and regs->orig_ax.

As for CLEAR_REGS, I also have reviewed it many times.  To me, it
equals clearing all registers although it doesn't clear ax, sp,
di, si.  In the comments, it says

   The lower registers are likely clobbered well before they could
   be put to use in a speculative execution gadget.

When using CLEAR_REGS for the INT80 thing, the %rsi will be cleared
explicitly when syscall_enter_from_user_mode() which has 2 arguments
is called.

"identical" is overstated. I will change the changelog to say their
behaviors are almost similar and the final result are the same when
the macro idtentry has the prolog.

Thanks
Lai