From: Greg Kroah-Hartman Date: Mon, 5 May 2025 09:29:52 +0000 (+0200) Subject: 5.15-stable patches X-Git-Tag: v5.15.182~65 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=183cfe9f6057da985251e36f28e64fee7e654d3f;p=thirdparty%2Fkernel%2Fstable-queue.git 5.15-stable patches added patches: kvm-x86-load-dr6-with-guest-value-only-before-entering-.vcpu_run-loop.patch net-sched-act_mirred-don-t-override-retval-if-we-already-lost-the-skb.patch --- diff --git a/queue-5.15/kvm-x86-load-dr6-with-guest-value-only-before-entering-.vcpu_run-loop.patch b/queue-5.15/kvm-x86-load-dr6-with-guest-value-only-before-entering-.vcpu_run-loop.patch new file mode 100644 index 0000000000..68424ecc51 --- /dev/null +++ b/queue-5.15/kvm-x86-load-dr6-with-guest-value-only-before-entering-.vcpu_run-loop.patch @@ -0,0 +1,228 @@ +From c2fee09fc167c74a64adb08656cb993ea475197e Mon Sep 17 00:00:00 2001 +From: Sean Christopherson +Date: Fri, 24 Jan 2025 17:18:33 -0800 +Subject: KVM: x86: Load DR6 with guest value only before entering .vcpu_run() loop + +From: Sean Christopherson + +commit c2fee09fc167c74a64adb08656cb993ea475197e upstream. + +Move the conditional loading of hardware DR6 with the guest's DR6 value +out of the core .vcpu_run() loop to fix a bug where KVM can load hardware +with a stale vcpu->arch.dr6. + +When the guest accesses a DR and host userspace isn't debugging the guest, +KVM disables DR interception and loads the guest's values into hardware on +VM-Enter and saves them on VM-Exit. This allows the guest to access DRs +at will, e.g. so that a sequence of DR accesses to configure a breakpoint +only generates one VM-Exit. + +For DR0-DR3, the logic/behavior is identical between VMX and SVM, and also +identical between KVM_DEBUGREG_BP_ENABLED (userspace debugging the guest) +and KVM_DEBUGREG_WONT_EXIT (guest using DRs), and so KVM handles loading +DR0-DR3 in common code, _outside_ of the core kvm_x86_ops.vcpu_run() loop. + +But for DR6, the guest's value doesn't need to be loaded into hardware for +KVM_DEBUGREG_BP_ENABLED, and SVM provides a dedicated VMCB field whereas +VMX requires software to manually load the guest value, and so loading the +guest's value into DR6 is handled by {svm,vmx}_vcpu_run(), i.e. is done +_inside_ the core run loop. + +Unfortunately, saving the guest values on VM-Exit is initiated by common +x86, again outside of the core run loop. If the guest modifies DR6 (in +hardware, when DR interception is disabled), and then the next VM-Exit is +a fastpath VM-Exit, KVM will reload hardware DR6 with vcpu->arch.dr6 and +clobber the guest's actual value. + +The bug shows up primarily with nested VMX because KVM handles the VMX +preemption timer in the fastpath, and the window between hardware DR6 +being modified (in guest context) and DR6 being read by guest software is +orders of magnitude larger in a nested setup. E.g. in non-nested, the +VMX preemption timer would need to fire precisely between #DB injection +and the #DB handler's read of DR6, whereas with a KVM-on-KVM setup, the +window where hardware DR6 is "dirty" extends all the way from L1 writing +DR6 to VMRESUME (in L1). + + L1's view: + ========== + + CPU 0/KVM-7289 [023] d.... 2925.640961: kvm_entry: vcpu 0 + A: L1 Writes DR6 + CPU 0/KVM-7289 [023] d.... 2925.640963: : Set DRs, DR6 = 0xffff0ff1 + + B: CPU 0/KVM-7289 [023] d.... 2925.640967: kvm_exit: vcpu 0 reason EXTERNAL_INTERRUPT intr_info 0x800000ec + + D: L1 reads DR6, arch.dr6 = 0 + CPU 0/KVM-7289 [023] d.... 2925.640969: : Sync DRs, DR6 = 0xffff0ff0 + + CPU 0/KVM-7289 [023] d.... 2925.640976: kvm_entry: vcpu 0 + L2 reads DR6, L1 disables DR interception + CPU 0/KVM-7289 [023] d.... 2925.640980: kvm_exit: vcpu 0 reason DR_ACCESS info1 0x0000000000000216 + CPU 0/KVM-7289 [023] d.... 2925.640983: kvm_entry: vcpu 0 + + CPU 0/KVM-7289 [023] d.... 2925.640983: : Set DRs, DR6 = 0xffff0ff0 + + L2 detects failure + CPU 0/KVM-7289 [023] d.... 2925.640987: kvm_exit: vcpu 0 reason HLT + L1 reads DR6 (confirms failure) + CPU 0/KVM-7289 [023] d.... 2925.640990: : Sync DRs, DR6 = 0xffff0ff0 + + L0's view: + ========== + L2 reads DR6, arch.dr6 = 0 + CPU 23/KVM-5046 [001] d.... 3410.005610: kvm_exit: vcpu 23 reason DR_ACCESS info1 0x0000000000000216 + CPU 23/KVM-5046 [001] ..... 3410.005610: kvm_nested_vmexit: vcpu 23 reason DR_ACCESS info1 0x0000000000000216 + + L2 => L1 nested VM-Exit + CPU 23/KVM-5046 [001] ..... 3410.005610: kvm_nested_vmexit_inject: reason: DR_ACCESS ext_inf1: 0x0000000000000216 + + CPU 23/KVM-5046 [001] d.... 3410.005610: kvm_entry: vcpu 23 + CPU 23/KVM-5046 [001] d.... 3410.005611: kvm_exit: vcpu 23 reason VMREAD + CPU 23/KVM-5046 [001] d.... 3410.005611: kvm_entry: vcpu 23 + CPU 23/KVM-5046 [001] d.... 3410.005612: kvm_exit: vcpu 23 reason VMREAD + CPU 23/KVM-5046 [001] d.... 3410.005612: kvm_entry: vcpu 23 + + L1 writes DR7, L0 disables DR interception + CPU 23/KVM-5046 [001] d.... 3410.005612: kvm_exit: vcpu 23 reason DR_ACCESS info1 0x0000000000000007 + CPU 23/KVM-5046 [001] d.... 3410.005613: kvm_entry: vcpu 23 + + L0 writes DR6 = 0 (arch.dr6) + CPU 23/KVM-5046 [001] d.... 3410.005613: : Set DRs, DR6 = 0xffff0ff0 + + A: + + B: CPU 23/KVM-5046 [001] d.... 3410.005614: kvm_exit: vcpu 23 reason PREEMPTION_TIMER + CPU 23/KVM-5046 [001] d.... 3410.005614: kvm_entry: vcpu 23 + + C: L0 writes DR6 = 0 (arch.dr6) + CPU 23/KVM-5046 [001] d.... 3410.005614: : Set DRs, DR6 = 0xffff0ff0 + + L1 => L2 nested VM-Enter + CPU 23/KVM-5046 [001] d.... 3410.005616: kvm_exit: vcpu 23 reason VMRESUME + + L0 reads DR6, arch.dr6 = 0 + +Reported-by: John Stultz +Closes: https://lkml.kernel.org/r/CANDhNCq5_F3HfFYABqFGCA1bPd_%2BxgNj-iDQhH4tDk%2Bwi8iZZg%40mail.gmail.com +Fixes: 375e28ffc0cf ("KVM: X86: Set host DR6 only on VMX and for KVM_DEBUGREG_WONT_EXIT") +Fixes: d67668e9dd76 ("KVM: x86, SVM: isolate vcpu->arch.dr6 from vmcb->save.dr6") +Cc: stable@vger.kernel.org +Cc: Jim Mattson +Tested-by: John Stultz +Link: https://lore.kernel.org/r/20250125011833.3644371-1-seanjc@google.com +Signed-off-by: Sean Christopherson +[jth: Handled conflicts with kvm_x86_ops reshuffle] +Signed-off-by: James Houghton +Signed-off-by: Greg Kroah-Hartman +--- + arch/x86/include/asm/kvm-x86-ops.h | 1 + + arch/x86/include/asm/kvm_host.h | 1 + + arch/x86/kvm/svm/svm.c | 13 ++++++------- + arch/x86/kvm/vmx/vmx.c | 11 +++++++---- + arch/x86/kvm/x86.c | 3 +++ + 5 files changed, 18 insertions(+), 11 deletions(-) + +--- a/arch/x86/include/asm/kvm-x86-ops.h ++++ b/arch/x86/include/asm/kvm-x86-ops.h +@@ -44,6 +44,7 @@ KVM_X86_OP(set_idt) + KVM_X86_OP(get_gdt) + KVM_X86_OP(set_gdt) + KVM_X86_OP(sync_dirty_debug_regs) ++KVM_X86_OP(set_dr6) + KVM_X86_OP(set_dr7) + KVM_X86_OP(cache_reg) + KVM_X86_OP(get_rflags) +--- a/arch/x86/include/asm/kvm_host.h ++++ b/arch/x86/include/asm/kvm_host.h +@@ -1344,6 +1344,7 @@ struct kvm_x86_ops { + void (*get_gdt)(struct kvm_vcpu *vcpu, struct desc_ptr *dt); + void (*set_gdt)(struct kvm_vcpu *vcpu, struct desc_ptr *dt); + void (*sync_dirty_debug_regs)(struct kvm_vcpu *vcpu); ++ void (*set_dr6)(struct kvm_vcpu *vcpu, unsigned long value); + void (*set_dr7)(struct kvm_vcpu *vcpu, unsigned long value); + void (*cache_reg)(struct kvm_vcpu *vcpu, enum kvm_reg reg); + unsigned long (*get_rflags)(struct kvm_vcpu *vcpu); +--- a/arch/x86/kvm/svm/svm.c ++++ b/arch/x86/kvm/svm/svm.c +@@ -1887,11 +1887,11 @@ static void new_asid(struct vcpu_svm *sv + svm->asid = sd->next_asid++; + } + +-static void svm_set_dr6(struct vcpu_svm *svm, unsigned long value) ++static void svm_set_dr6(struct kvm_vcpu *vcpu, unsigned long value) + { +- struct vmcb *vmcb = svm->vmcb; ++ struct vmcb *vmcb = to_svm(vcpu)->vmcb; + +- if (svm->vcpu.arch.guest_state_protected) ++ if (vcpu->arch.guest_state_protected) + return; + + if (unlikely(value != vmcb->save.dr6)) { +@@ -3851,10 +3851,8 @@ static __no_kcsan fastpath_t svm_vcpu_ru + * Run with all-zero DR6 unless needed, so that we can get the exact cause + * of a #DB. + */ +- if (unlikely(vcpu->arch.switch_db_regs & KVM_DEBUGREG_WONT_EXIT)) +- svm_set_dr6(svm, vcpu->arch.dr6); +- else +- svm_set_dr6(svm, DR6_ACTIVE_LOW); ++ if (likely(!(vcpu->arch.switch_db_regs & KVM_DEBUGREG_WONT_EXIT))) ++ svm_set_dr6(vcpu, DR6_ACTIVE_LOW); + + clgi(); + kvm_load_guest_xsave_state(vcpu); +@@ -4631,6 +4629,7 @@ static struct kvm_x86_ops svm_x86_ops __ + .set_idt = svm_set_idt, + .get_gdt = svm_get_gdt, + .set_gdt = svm_set_gdt, ++ .set_dr6 = svm_set_dr6, + .set_dr7 = svm_set_dr7, + .sync_dirty_debug_regs = svm_sync_dirty_debug_regs, + .cache_reg = svm_cache_reg, +--- a/arch/x86/kvm/vmx/vmx.c ++++ b/arch/x86/kvm/vmx/vmx.c +@@ -5249,6 +5249,12 @@ static void vmx_sync_dirty_debug_regs(st + set_debugreg(DR6_RESERVED, 6); + } + ++static void vmx_set_dr6(struct kvm_vcpu *vcpu, unsigned long val) ++{ ++ lockdep_assert_irqs_disabled(); ++ set_debugreg(vcpu->arch.dr6, 6); ++} ++ + static void vmx_set_dr7(struct kvm_vcpu *vcpu, unsigned long val) + { + vmcs_writel(GUEST_DR7, val); +@@ -6839,10 +6845,6 @@ static fastpath_t vmx_vcpu_run(struct kv + vmx->loaded_vmcs->host_state.cr4 = cr4; + } + +- /* When KVM_DEBUGREG_WONT_EXIT, dr6 is accessible in guest. */ +- if (unlikely(vcpu->arch.switch_db_regs & KVM_DEBUGREG_WONT_EXIT)) +- set_debugreg(vcpu->arch.dr6, 6); +- + /* When single-stepping over STI and MOV SS, we must clear the + * corresponding interruptibility bits in the guest state. Otherwise + * vmentry fails as it then expects bit 14 (BS) in pending debug +@@ -7777,6 +7779,7 @@ static struct kvm_x86_ops vmx_x86_ops __ + .set_idt = vmx_set_idt, + .get_gdt = vmx_get_gdt, + .set_gdt = vmx_set_gdt, ++ .set_dr6 = vmx_set_dr6, + .set_dr7 = vmx_set_dr7, + .sync_dirty_debug_regs = vmx_sync_dirty_debug_regs, + .cache_reg = vmx_cache_reg, +--- a/arch/x86/kvm/x86.c ++++ b/arch/x86/kvm/x86.c +@@ -9963,6 +9963,9 @@ static int vcpu_enter_guest(struct kvm_v + set_debugreg(vcpu->arch.eff_db[1], 1); + set_debugreg(vcpu->arch.eff_db[2], 2); + set_debugreg(vcpu->arch.eff_db[3], 3); ++ /* When KVM_DEBUGREG_WONT_EXIT, dr6 is accessible in guest. */ ++ if (unlikely(vcpu->arch.switch_db_regs & KVM_DEBUGREG_WONT_EXIT)) ++ static_call(kvm_x86_set_dr6)(vcpu, vcpu->arch.dr6); + } else if (unlikely(hw_breakpoint_active())) { + set_debugreg(0, 7); + } diff --git a/queue-5.15/net-sched-act_mirred-don-t-override-retval-if-we-already-lost-the-skb.patch b/queue-5.15/net-sched-act_mirred-don-t-override-retval-if-we-already-lost-the-skb.patch new file mode 100644 index 0000000000..db2f0de605 --- /dev/null +++ b/queue-5.15/net-sched-act_mirred-don-t-override-retval-if-we-already-lost-the-skb.patch @@ -0,0 +1,89 @@ +From 166c2c8a6a4dc2e4ceba9e10cfe81c3e469e3210 Mon Sep 17 00:00:00 2001 +From: Jakub Kicinski +Date: Thu, 15 Feb 2024 06:33:46 -0800 +Subject: net/sched: act_mirred: don't override retval if we already lost the skb + +From: Jakub Kicinski + +commit 166c2c8a6a4dc2e4ceba9e10cfe81c3e469e3210 upstream. + +If we're redirecting the skb, and haven't called tcf_mirred_forward(), +yet, we need to tell the core to drop the skb by setting the retcode +to SHOT. If we have called tcf_mirred_forward(), however, the skb +is out of our hands and returning SHOT will lead to UaF. + +Move the retval override to the error path which actually need it. + +Reviewed-by: Michal Swiatkowski +Fixes: e5cf1baf92cb ("act_mirred: use TC_ACT_REINSERT when possible") +Signed-off-by: Jakub Kicinski +Acked-by: Jamal Hadi Salim +Signed-off-by: David S. Miller +[Minor conflict resolved due to code context change.] +Signed-off-by: Jianqi Ren +Signed-off-by: He Zhe +Signed-off-by: Greg Kroah-Hartman +--- + net/sched/act_mirred.c | 22 +++++++++++++--------- + 1 file changed, 13 insertions(+), 9 deletions(-) + +--- a/net/sched/act_mirred.c ++++ b/net/sched/act_mirred.c +@@ -254,31 +254,31 @@ static int tcf_mirred_act(struct sk_buff + + m_mac_header_xmit = READ_ONCE(m->tcfm_mac_header_xmit); + m_eaction = READ_ONCE(m->tcfm_eaction); ++ is_redirect = tcf_mirred_is_act_redirect(m_eaction); + retval = READ_ONCE(m->tcf_action); + dev = rcu_dereference_bh(m->tcfm_dev); + if (unlikely(!dev)) { + pr_notice_once("tc mirred: target device is gone\n"); +- goto out; ++ goto err_cant_do; + } + + if (unlikely(!(dev->flags & IFF_UP)) || !netif_carrier_ok(dev)) { + net_notice_ratelimited("tc mirred to Houston: device %s is down\n", + dev->name); +- goto out; ++ goto err_cant_do; + } + + /* we could easily avoid the clone only if called by ingress and clsact; + * since we can't easily detect the clsact caller, skip clone only for + * ingress - that covers the TC S/W datapath. + */ +- is_redirect = tcf_mirred_is_act_redirect(m_eaction); + at_ingress = skb_at_tc_ingress(skb); + use_reinsert = at_ingress && is_redirect && + tcf_mirred_can_reinsert(retval); + if (!use_reinsert) { + skb2 = skb_clone(skb, GFP_ATOMIC); + if (!skb2) +- goto out; ++ goto err_cant_do; + } + + want_ingress = tcf_mirred_act_wants_ingress(m_eaction); +@@ -321,12 +321,16 @@ static int tcf_mirred_act(struct sk_buff + } + + err = tcf_mirred_forward(want_ingress, skb2); +- if (err) { +-out: ++ if (err) + tcf_action_inc_overlimit_qstats(&m->common); +- if (tcf_mirred_is_act_redirect(m_eaction)) +- retval = TC_ACT_SHOT; +- } ++ __this_cpu_dec(mirred_nest_level); ++ ++ return retval; ++ ++err_cant_do: ++ if (is_redirect) ++ retval = TC_ACT_SHOT; ++ tcf_action_inc_overlimit_qstats(&m->common); + __this_cpu_dec(mirred_nest_level); + + return retval; diff --git a/queue-5.15/series b/queue-5.15/series index f680d8ac5f..80051bbe8c 100644 --- a/queue-5.15/series +++ b/queue-5.15/series @@ -13,3 +13,5 @@ dm-always-update-the-array-size-in-realloc_argv-on-success.patch iommu-amd-fix-potential-buffer-overflow-in-parse_ivrs_acpihid.patch iommu-vt-d-apply-quirk_iommu_igfx-for-8086-0044-qm57-qs57.patch tracing-fix-oob-write-in-trace_seq_to_buffer.patch +kvm-x86-load-dr6-with-guest-value-only-before-entering-.vcpu_run-loop.patch +net-sched-act_mirred-don-t-override-retval-if-we-already-lost-the-skb.patch