Messages in this thread | | | From | Lai Jiangshan <> | Date | Mon, 25 Apr 2022 21:25:18 +0800 | Subject | Re: [PATCH V5 7/7] x86/entry: Use idtentry macro for entry_INT80_compat |
| |
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
| |