diff options
Diffstat (limited to '0046-x86-paging-Delete-update_cr3-s-do_locking-parameter.patch')
| -rw-r--r-- | 0046-x86-paging-Delete-update_cr3-s-do_locking-parameter.patch | 161 |
1 files changed, 161 insertions, 0 deletions
diff --git a/0046-x86-paging-Delete-update_cr3-s-do_locking-parameter.patch b/0046-x86-paging-Delete-update_cr3-s-do_locking-parameter.patch new file mode 100644 index 0000000..ce397a1 --- /dev/null +++ b/0046-x86-paging-Delete-update_cr3-s-do_locking-parameter.patch @@ -0,0 +1,161 @@ +From bf70ce8b3449c49eb828d5b1f4934a49b00fef35 Mon Sep 17 00:00:00 2001 +From: Andrew Cooper <andrew.cooper3@citrix.com> +Date: Wed, 20 Sep 2023 20:06:53 +0100 +Subject: [PATCH 46/67] x86/paging: Delete update_cr3()'s do_locking parameter + +Nicola reports that the XSA-438 fix introduced new MISRA violations because of +some incidental tidying it tried to do. The parameter is useless, so resolve +the MISRA regression by removing it. + +hap_update_cr3() discards the parameter entirely, while sh_update_cr3() uses +it to distinguish internal and external callers and therefore whether the +paging lock should be taken. + +However, we have paging_lock_recursive() for this purpose, which also avoids +the ability for the shadow internal callers to accidentally not hold the lock. + +Fixes: fb0ff49fe9f7 ("x86/shadow: defer releasing of PV's top-level shadow reference") +Reported-by: Nicola Vetrini <nicola.vetrini@bugseng.com> +Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> +Reviewed-by: Jan Beulich <jbeulich@suse.com> +Release-acked-by: Henry Wang <Henry.Wang@arm.com> +(cherry picked from commit e71157d1ac2a7fbf413130663cf0a93ff9fbcf7e) +--- + xen/arch/x86/include/asm/paging.h | 5 ++--- + xen/arch/x86/mm/hap/hap.c | 5 ++--- + xen/arch/x86/mm/shadow/common.c | 2 +- + xen/arch/x86/mm/shadow/multi.c | 17 ++++++++--------- + xen/arch/x86/mm/shadow/none.c | 3 +-- + 5 files changed, 14 insertions(+), 18 deletions(-) + +diff --git a/xen/arch/x86/include/asm/paging.h b/xen/arch/x86/include/asm/paging.h +index 94c590f31a..809ff35d9a 100644 +--- a/xen/arch/x86/include/asm/paging.h ++++ b/xen/arch/x86/include/asm/paging.h +@@ -138,8 +138,7 @@ struct paging_mode { + paddr_t ga, uint32_t *pfec, + unsigned int *page_order); + #endif +- pagetable_t (*update_cr3 )(struct vcpu *v, bool do_locking, +- bool noflush); ++ pagetable_t (*update_cr3 )(struct vcpu *v, bool noflush); + void (*update_paging_modes )(struct vcpu *v); + bool (*flush_tlb )(const unsigned long *vcpu_bitmap); + +@@ -312,7 +311,7 @@ static inline unsigned long paging_ga_to_gfn_cr3(struct vcpu *v, + * as the value to load into the host CR3 to schedule this vcpu */ + static inline pagetable_t paging_update_cr3(struct vcpu *v, bool noflush) + { +- return paging_get_hostmode(v)->update_cr3(v, 1, noflush); ++ return paging_get_hostmode(v)->update_cr3(v, noflush); + } + + /* Update all the things that are derived from the guest's CR0/CR3/CR4. +diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c +index 57a19c3d59..3ad39a7dd7 100644 +--- a/xen/arch/x86/mm/hap/hap.c ++++ b/xen/arch/x86/mm/hap/hap.c +@@ -739,8 +739,7 @@ static bool cf_check hap_invlpg(struct vcpu *v, unsigned long linear) + return 1; + } + +-static pagetable_t cf_check hap_update_cr3( +- struct vcpu *v, bool do_locking, bool noflush) ++static pagetable_t cf_check hap_update_cr3(struct vcpu *v, bool noflush) + { + v->arch.hvm.hw_cr[3] = v->arch.hvm.guest_cr[3]; + hvm_update_guest_cr3(v, noflush); +@@ -826,7 +825,7 @@ static void cf_check hap_update_paging_modes(struct vcpu *v) + } + + /* CR3 is effectively updated by a mode change. Flush ASIDs, etc. */ +- hap_update_cr3(v, 0, false); ++ hap_update_cr3(v, false); + + unlock: + paging_unlock(d); +diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c +index c0940f939e..18714dbd02 100644 +--- a/xen/arch/x86/mm/shadow/common.c ++++ b/xen/arch/x86/mm/shadow/common.c +@@ -2579,7 +2579,7 @@ static void sh_update_paging_modes(struct vcpu *v) + } + #endif /* OOS */ + +- v->arch.paging.mode->update_cr3(v, 0, false); ++ v->arch.paging.mode->update_cr3(v, false); + } + + void cf_check shadow_update_paging_modes(struct vcpu *v) +diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c +index c92b354a78..e54a507b54 100644 +--- a/xen/arch/x86/mm/shadow/multi.c ++++ b/xen/arch/x86/mm/shadow/multi.c +@@ -2506,7 +2506,7 @@ static int cf_check sh_page_fault( + * In any case, in the PAE case, the ASSERT is not true; it can + * happen because of actions the guest is taking. */ + #if GUEST_PAGING_LEVELS == 3 +- v->arch.paging.mode->update_cr3(v, 0, false); ++ v->arch.paging.mode->update_cr3(v, false); + #else + ASSERT(d->is_shutting_down); + #endif +@@ -3224,17 +3224,13 @@ static void cf_check sh_detach_old_tables(struct vcpu *v) + } + } + +-static pagetable_t cf_check sh_update_cr3(struct vcpu *v, bool do_locking, +- bool noflush) ++static pagetable_t cf_check sh_update_cr3(struct vcpu *v, bool noflush) + /* Updates vcpu->arch.cr3 after the guest has changed CR3. + * Paravirtual guests should set v->arch.guest_table (and guest_table_user, + * if appropriate). + * HVM guests should also make sure hvm_get_guest_cntl_reg(v, 3) works; + * this function will call hvm_update_guest_cr(v, 3) to tell them where the + * shadow tables are. +- * If do_locking != 0, assume we are being called from outside the +- * shadow code, and must take and release the paging lock; otherwise +- * that is the caller's responsibility. + */ + { + struct domain *d = v->domain; +@@ -3252,7 +3248,11 @@ static pagetable_t cf_check sh_update_cr3(struct vcpu *v, bool do_locking, + return old_entry; + } + +- if ( do_locking ) paging_lock(v->domain); ++ /* ++ * This is used externally (with the paging lock not taken) and internally ++ * by the shadow code (with the lock already taken). ++ */ ++ paging_lock_recursive(v->domain); + + #if (SHADOW_OPTIMIZATIONS & SHOPT_OUT_OF_SYNC) + /* Need to resync all the shadow entries on a TLB flush. Resync +@@ -3480,8 +3480,7 @@ static pagetable_t cf_check sh_update_cr3(struct vcpu *v, bool do_locking, + shadow_sync_other_vcpus(v); + #endif + +- /* Release the lock, if we took it (otherwise it's the caller's problem) */ +- if ( do_locking ) paging_unlock(v->domain); ++ paging_unlock(v->domain); + + return old_entry; + } +diff --git a/xen/arch/x86/mm/shadow/none.c b/xen/arch/x86/mm/shadow/none.c +index 743c0ffb85..7e4e386cd0 100644 +--- a/xen/arch/x86/mm/shadow/none.c ++++ b/xen/arch/x86/mm/shadow/none.c +@@ -52,8 +52,7 @@ static unsigned long cf_check _gva_to_gfn( + } + #endif + +-static pagetable_t cf_check _update_cr3(struct vcpu *v, bool do_locking, +- bool noflush) ++static pagetable_t cf_check _update_cr3(struct vcpu *v, bool noflush) + { + ASSERT_UNREACHABLE(); + return pagetable_null(); +-- +2.44.0 + |