[PATCH] arm64: entry: Simplify tramp_alias macro and tramp_exit routine
Ard Biesheuvel
ardb at kernel.org
Sat Apr 8 01:35:37 PDT 2023
On Thu, 6 Apr 2023 at 17:30, Ard Biesheuvel <ardb at kernel.org> wrote:
>
> On Thu, 6 Apr 2023 at 16:39, Will Deacon <will at kernel.org> wrote:
> >
> > Hey Ard,
> >
> > On Tue, Mar 07, 2023 at 12:57:27PM +0100, Ard Biesheuvel wrote:
> > > The tramp_alias macro constructs the virtual alias of a symbol in the
> > > trampoline text mapping, based on its kernel text address, and does so
> > > in a way that is more convoluted than necessary. So let's simplify it.
> > >
> > > Also, now that the address of the vector table is kept in a per-CPU
> > > variable, there is no need to defer the load and the assignment of
> > > VBAR_EL1 to tramp_exit(). Given that tramp_alias no longer needs a temp
> > > register, this means we can restore X30 earlier as well, and only leave
> > > X29 for tramp_exit() to restore.
> > >
> > > Finally, merge the native and compat versions of tramp_exit() - the only
> > > difference is whether X29 is reloaded from FAR_EL1, and we can just zero
> > > it conditionally rather than having two distinct code paths.
> > >
> > > While at it, give some related symbols static linkage, considering that
> > > they are only referenced from the object file that defines them.
> > >
> > > Cc: James Morse <james.morse at arm.com>
> > > Signed-off-by: Ard Biesheuvel <ardb at kernel.org>
> > > ---
> > > arch/arm64/kernel/entry.S | 58 ++++++++------------
> > > 1 file changed, 22 insertions(+), 36 deletions(-)
> >
> > [...]
> >
> > > @@ -768,7 +753,7 @@ alternative_else_nop_endif
> > > */
> > > .pushsection ".entry.tramp.text", "ax"
> > > .align 11
> > > -SYM_CODE_START_NOALIGN(tramp_vectors)
> > > +SYM_CODE_START_LOCAL_NOALIGN(tramp_vectors)
> > > #ifdef CONFIG_MITIGATE_SPECTRE_BRANCH_HISTORY
> > > generate_tramp_vector kpti=1, bhb=BHB_MITIGATION_LOOP
> > > generate_tramp_vector kpti=1, bhb=BHB_MITIGATION_FW
> > > @@ -777,13 +762,14 @@ SYM_CODE_START_NOALIGN(tramp_vectors)
> > > generate_tramp_vector kpti=1, bhb=BHB_MITIGATION_NONE
> > > SYM_CODE_END(tramp_vectors)
> > >
> > > -SYM_CODE_START(tramp_exit_native)
> > > - tramp_exit
> > > -SYM_CODE_END(tramp_exit_native)
> > > -
> > > -SYM_CODE_START(tramp_exit_compat)
> > > - tramp_exit 32
> > > -SYM_CODE_END(tramp_exit_compat)
> > > +SYM_CODE_START_LOCAL(tramp_exit)
> > > + // Entered with Z flag set if task is native
> > > + tramp_unmap_kernel x29
> > > + mrs x29, far_el1 // restore x29
> > > + csel x29, x29, xzr, eq // clear x29 for compat
> >
> > Why do we care about zeroing x29 for a compat task?
> >
>
> The only reason I could think of is that it we don't want to leak
> anything, in case the value might be exposed in some way? (e.,g,
> ptrace from another, 64-bit process?)
BTW just realized that I failed to mention in the commit log that this
removes a memory access from the the ret_to_user path:
By moving this
- tramp_data_read_var x30, this_cpu_vector
- get_this_cpu_offset x29
- ldr x30, [x30, x29]
out of the trampoline and into the ordinary kernel mapping, it can be changed to
+4: ldr_this_cpu x30, this_cpu_vector, x29
which removes one of the two LDR instructions.
More information about the linux-arm-kernel
mailing list