Re: [PATCH] arm64: Fix compile error with KVM and !HARDEN_BRANCH_PREDICTOR

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On Fri, 29 Nov 2019 at 09:04, Marc Zyngier <maz@kernel.org> wrote:
>
> On 2019-11-29 07:25, Ard Biesheuvel wrote:
> > On Fri, 29 Nov 2019 at 08:21, Marc Zyngier <maz@kernel.org> wrote:
> >>
> >> On Thu, 28 Nov 2019 17:20:20 +0000,
> >>
> >> [fixing Will's email address]
> >>
> >> Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >> >
> >> > (+ Marc)
> >> >
> >> >
> >> >
> >> > On Wed, 27 Nov 2019 at 19:10, <minyard@acm.org> wrote:
> >> > >
> >> > > From: Corey Minyard <cminyard@mvista.com>
> >> > >
> >> > > When compiling with KVM enabled and without
> >> HARDEN_BRANCH_PREDICTOR,
> >> > > the following compile error happens:
> >> > >
> >> > > arch/arm64/kernel/cpu_errata.c:92:23:
> >> > > error: '__bp_harden_hyp_vecs_start' undeclared (first use in
> >> this function);
> >> > > did you mean 'hyp_vecs_start'?
> >> > >   void *dst = lm_alias(__bp_harden_hyp_vecs_start + slot *
> >> SZ_2K);
> >> > >
> >> > > Some ifdefs were removed by 3e91f3eacc91d9 "arm64: Always enable
> >> > > spectre-v2 vulnerability detection" for
> >> CONFIG_HARDEN_BRANCH_PREDICTOR,
> >> > > but __bp_harden_hyp_vecs_start is only defined if that config is
> >> > > enabled.
> >> > >
> >> > > Add CONFIG_HARDEN_BRANCH_PREDICTOR to the #if that has
> >> CONFIG_KVM,
> >> > > It looks like you need both of those for that code to be valid.
> >> > >
> >> > > Fixes: 3e91f3eacc91d9 "arm64: Always enable spectre-v2
> >> vulnerability detection"
> >> > > Cc: Andre Przywara <andre.przywara@arm.com>
> >> > > Cc: Catalin Marinas <catalin.marinas@arm.com>
> >> > > Cc: Stefan Wahren <stefan.wahren@i2se.com>
> >> > > Cc: Will Deacon <will.deacon@arm.com>
> >> > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> > > Signed-off-by: Corey Minyard <cminyard@mvista.com>
> >> > > ---
> >> > > This is for 4.14, I'm not sure if it is needed for other
> >> kernels.
> >> > >
> >> > > It is not needed in master because a new config item was added,
> >> > > CONFIG_KVM_INDIRECT_VECTORS, that depends on KVM and
> >> > > HARDEN_BRANCH_PREDICTOR being configured.  I looked at pulling
> >> the
> >> > > patches that add the required changes, and they make a lot of
> >> > > changes.  This change is the simple fix, but I'm not sure if we
> >> want to
> >> > > pull all those other changes into 4.14 and whatever other
> >> kernels
> >> > > are required.
> >> > >
> >> >
> >> > I agree that backporting this cleanly is going to be problematic,
> >> > since it pulls in the entire EL2 VA randomization feature with all
> >> its
> >> > prerequisites.
> >> >
> >> > Backporting the following set could be done fairly cleanly, and
> >> fixes
> >> > the problem at build time, but unfortunately, it causes a boot
> >> time
> >> > crash (see below)
> >> >
> >> > 97eca4d2bfec (HEAD -> linux-4.14.y) arm64: Move the content of
> >> bpi.S
> >> > to hyp-entry.S
> >> > 514dd33114c6 arm64: Make BP hardening slot counter available
> >> > d7ddf3ae9470 arm64; insn: Add encoder for the EXTR instruction
> >> > c0b2c4e56e10 arm64: KVM: Introduce EL2 VA randomisation
> >> > 56ab0a87c737 arm64: KVM: Dynamically compute the HYP VA mask
> >> > d92c02628dfb arm64: KVM: Allow far branches from vector slots to
> >> the
> >> > main vectors
> >> > 7adec01c9674 arm64: cpufeatures: Drop the ARM64_HYP_OFFSET_LOW
> >> feature flag
> >> > 1095e4fc3134 arm64: KVM: Move stashing of x0/x1 into the vector
> >> code itself
> >> > bb2e1aceb423 arm64: KVM: Dynamically patch the kernel/hyp VA mask
> >> > 6f0ccfc451be arm64: KVM: Reserve 4 additional instructions in the
> >> BPI template
> >> > bf425ffee07a arm64: insn: Add encoder for bitwise operations using
> >> literals
> >> > 41dda94d1a9f arm64: insn: Add N immediate encoding
> >> > 3225668ebe00 arm64: KVM: Move BP hardening vectors into .hyp.text
> >> section
> >> >
> >> > Marc?
> >>
> >> You need at least these:
> >>
> >> 1bb32a44aea1 KVM: arm/arm64: Keep GICv2 HYP VAs in
> >> kvm_vgic_global_state
> >> 44a497abd621 KVM: arm/arm64: Do not use kern_hyp_va() with
> >> kvm_vgic_global_state
> >>
> >
> > Yeah, I only did a fairly mechanical backport based on the actual
> > diffs depend on each other, but I did spot those two as possibly
> > related.
> >
> >> But that's definitely not enough to fix the crash.
> >>
> >> >
> >> > [    0.062126] CPU: All CPU(s) started at EL1
> >> > [    0.063109] alternatives: patching kernel code
> >> > [    0.064228] random: get_random_u64 called from
> >> > compute_layout+0x94/0xe8 with crng_init=0
> >> > [    0.066313] aarch64_insn_gen_add_sub_imm: invalid immediate
> >> encoding 1904640
> >>
> >> OK, that one is really bizarre. This value (tag_val) is supposed to
> >> be
> >> a small value (only 12 significant bits out of 24 at any given
> >> time),
> >> and it is not (0x1D1000). So somehow compute_instruction() is not
> >> doing the right thing.
> >>
> >> Do you have a tree somewhere with this patches?
> >>
> >
> > Sure, thanks for having a look.
> >
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=arm64-v4.14-backport%2b%2b
>
> OK, I tracked it down to this missing patch:
>
> 11d764079c9f arm64: insn: Allow ADD/SUB (immediate) with LSL #12
>
> I haven't tryied to boot the whole thing as a host though, only
> tested it as a guest.
>
> > Another thing I found bizarre is that we actually run this code when
> > all CPUs boot at EL1. Or is that intended?
>
> It is so that I can debug the whole thing in a guest! ;-)
>

OK, I have prepared a branch that carries all these patches in the
right order with the upstream commit reference added to the commit
log. I'll send this out for review first, before actually sending it
to -stable, unless anyone feels that this is a bad idea.

arm64: KVM: Move BP hardening vectors into .hyp.text section
arm64: insn: Add N immediate encoding
arm64: insn: Add encoder for bitwise operations using literals
arm64: KVM: Dynamically patch the kernel/hyp VA mask
arm64: cpufeatures: Drop the ARM64_HYP_OFFSET_LOW feature flag
arm64; insn: Add encoder for the EXTR instruction
arm64: insn: Allow ADD/SUB (immediate) with LSL #12
arm64: KVM: Dynamically compute the HYP VA mask
arm64: KVM: Introduce EL2 VA randomisation
arm64: KVM: Move stashing of x0/x1 into the vector code itself
arm64: KVM: Reserve 4 additional instructions in the BPI template
arm64: KVM: Allow far branches from vector slots to the main vectors
arm64: Make BP hardening slot counter available
arm64: Move the content of bpi.S to hyp-entry.S

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel