As all function pointer calls are resoved to direct calls on boot, clobber the
endbr64 instructions too to make life harder for an attacker which has managed
to hijack a function pointer.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
---
 xen/arch/x86/cpu/vpmu_amd.c   | 2 +-
 xen/arch/x86/cpu/vpmu_intel.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/cpu/vpmu_amd.c b/xen/arch/x86/cpu/vpmu_amd.c
index 903fe1887ef0..e26f4f584e88 100644
--- a/xen/arch/x86/cpu/vpmu_amd.c
+++ b/xen/arch/x86/cpu/vpmu_amd.c
@@ -518,7 +518,7 @@ static int svm_vpmu_initialise(struct vcpu *v)
     return 0;
 }
 
-static const struct arch_vpmu_ops __initconstrel amd_vpmu_ops = {
+static struct arch_vpmu_ops __initdata_cf_clobber amd_vpmu_ops = {
     .initialise = svm_vpmu_initialise,
     .do_wrmsr = amd_vpmu_do_wrmsr,
     .do_rdmsr = amd_vpmu_do_rdmsr,
diff --git a/xen/arch/x86/cpu/vpmu_intel.c b/xen/arch/x86/cpu/vpmu_intel.c
index 076882c615f4..98a93d1f3c41 100644
--- a/xen/arch/x86/cpu/vpmu_intel.c
+++ b/xen/arch/x86/cpu/vpmu_intel.c
@@ -880,7 +880,7 @@ static int vmx_vpmu_initialise(struct vcpu *v)
     return 0;
 }
 
-static const struct arch_vpmu_ops __initconstrel core2_vpmu_ops = {
+static struct arch_vpmu_ops __initdata_cf_clobber core2_vpmu_ops = {
     .initialise = vmx_vpmu_initialise,
     .do_wrmsr = core2_vpmu_do_wrmsr,
     .do_rdmsr = core2_vpmu_do_rdmsr,
-- 
2.11.0

On 30.11.2021 23:05, Andrew Cooper wrote:
> --- a/xen/arch/x86/cpu/vpmu_amd.c
> +++ b/xen/arch/x86/cpu/vpmu_amd.c
> @@ -518,7 +518,7 @@ static int svm_vpmu_initialise(struct vcpu *v)
>      return 0;
>  }
>  
> -static const struct arch_vpmu_ops __initconstrel amd_vpmu_ops = {
> +static struct arch_vpmu_ops __initdata_cf_clobber amd_vpmu_ops = {

This depends on an uncommitted patch (introducing the annotation)
and hence is a little difficult to review without a pointer to that
patch (which doesn't look to have "cf_" in its subject, and I didn't
recall anything else to search for in my mailbox). The main question
I see here is whether it's warranted to drop the const: I'd like to
retain it if at all possible, just to document that the structure
doesn't get modified at runtime (read: initialization time).

Later... I've spotted it in my to-be-reviewed folder; it's
"x86/altcall: Optimise away endbr64 instruction where possible".
There's no discussion there either about const. Iirc the reason we
need __initconstrel alongside __initconst is for older tool chains
complaining about section conflicts. If that's right, for CET-IBT
we need relatively new tool chains anyway. Hence the mixing of
const and non-const within a section may not be an issue. IOW with
newer tool chains all could go in a single section; we'd need two
separate annotations only for older tool chains (falling back to
__initdata or __initconstrel). Of course at that point it might be
easier to have both .init.data.cf_clobber and
.init.rodata.cf_clobber in the first place, grouped together by
the linker script.

Jan