| Message ID | 20211130220502.27624-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/vPMU: adjustements to vendor hooks treatment | expand |
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
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,
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(-)