Patch "x86/hyperv: Properly deal with empty cpumasks in hyperv_flush_tlb_multi()" has been added to the 5.15-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This is a note to let you know that I've just added the patch titled

    x86/hyperv: Properly deal with empty cpumasks in hyperv_flush_tlb_multi()

to the 5.15-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     x86-hyperv-properly-deal-with-empty-cpumasks-in-hype.patch
and it can be found in the queue-5.15 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.



commit a06817129afef53ebfa4ce59b9eb25fa74f343ef
Author: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
Date:   Thu Jan 6 10:46:11 2022 +0100

    x86/hyperv: Properly deal with empty cpumasks in hyperv_flush_tlb_multi()
    
    [ Upstream commit 51500b71d500f251037ed339047a4d9e7d7e295b ]
    
    KASAN detected the following issue:
    
     BUG: KASAN: slab-out-of-bounds in hyperv_flush_tlb_multi+0xf88/0x1060
     Read of size 4 at addr ffff8880011ccbc0 by task kcompactd0/33
    
     CPU: 1 PID: 33 Comm: kcompactd0 Not tainted 5.14.0-39.el9.x86_64+debug #1
     Hardware name: Microsoft Corporation Virtual Machine/Virtual Machine,
         BIOS Hyper-V UEFI Release v4.0 12/17/2019
     Call Trace:
      dump_stack_lvl+0x57/0x7d
      print_address_description.constprop.0+0x1f/0x140
      ? hyperv_flush_tlb_multi+0xf88/0x1060
      __kasan_report.cold+0x7f/0x11e
      ? hyperv_flush_tlb_multi+0xf88/0x1060
      kasan_report+0x38/0x50
      hyperv_flush_tlb_multi+0xf88/0x1060
      flush_tlb_mm_range+0x1b1/0x200
      ptep_clear_flush+0x10e/0x150
    ...
     Allocated by task 0:
      kasan_save_stack+0x1b/0x40
      __kasan_kmalloc+0x7c/0x90
      hv_common_init+0xae/0x115
      hyperv_init+0x97/0x501
      apic_intr_mode_init+0xb3/0x1e0
      x86_late_time_init+0x92/0xa2
      start_kernel+0x338/0x3eb
      secondary_startup_64_no_verify+0xc2/0xcb
    
     The buggy address belongs to the object at ffff8880011cc800
      which belongs to the cache kmalloc-1k of size 1024
     The buggy address is located 960 bytes inside of
      1024-byte region [ffff8880011cc800, ffff8880011ccc00)
    
    'hyperv_flush_tlb_multi+0xf88/0x1060' points to
    hv_cpu_number_to_vp_number() and '960 bytes' means we're trying to get
    VP_INDEX for CPU#240. 'nr_cpus' here is exactly 240 so we're trying to
    access past hv_vp_index's last element. This can (and will) happen
    when 'cpus' mask is empty and cpumask_last() will return '>=nr_cpus'.
    
    Commit ad0a6bad4475 ("x86/hyperv: check cpu mask after interrupt has
    been disabled") tried to deal with empty cpumask situation but
    apparently didn't fully fix the issue.
    
    'cpus' cpumask which is passed to hyperv_flush_tlb_multi() is
    'mm_cpumask(mm)' (which is '&mm->cpu_bitmap'). This mask changes every
    time the particular mm is scheduled/unscheduled on some CPU (see
    switch_mm_irqs_off()), disabling IRQs on the CPU which is performing remote
    TLB flush has zero influence on whether the particular process can get
    scheduled/unscheduled on _other_ CPUs so e.g. in the case where the mm was
    scheduled on one other CPU and got unscheduled during
    hyperv_flush_tlb_multi()'s execution will lead to cpumask becoming empty.
    
    It doesn't seem that there's a good way to protect 'mm_cpumask(mm)'
    from changing during hyperv_flush_tlb_multi()'s execution. It would be
    possible to copy it in the very beginning of the function but this is a
    waste. It seems we can deal with changing cpumask just fine.
    
    When 'cpus' cpumask changes during hyperv_flush_tlb_multi()'s
    execution, there are two possible issues:
    - 'Under-flushing': we will not flush TLB on a CPU which got added to
    the mask while hyperv_flush_tlb_multi() was already running. This is
    not a problem as this is equal to mm getting scheduled on that CPU
    right after TLB flush.
    - 'Over-flushing': we may flush TLB on a CPU which is already cleared
    from the mask. First, extra TLB flush preserves correctness. Second,
    Hyper-V's TLB flush hypercall takes 'mm->pgd' argument so Hyper-V may
    avoid the flush if CR3 doesn't match.
    
    Fix the immediate issue with cpumask_last()/hv_cpu_number_to_vp_number()
    and remove the pointless cpumask_empty() check from the beginning of the
    function as it really doesn't protect anything. Also, avoid the hypercall
    altogether when 'flush->processor_mask' ends up being empty.
    
    Fixes: ad0a6bad4475 ("x86/hyperv: check cpu mask after interrupt has been disabled")
    Signed-off-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
    Reviewed-by: Michael Kelley <mikelley@xxxxxxxxxxxxx>
    Link: https://lore.kernel.org/r/20220106094611.1404218-1-vkuznets@xxxxxxxxxx
    Signed-off-by: Wei Liu <wei.liu@xxxxxxxxxx>
    Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>

diff --git a/arch/x86/hyperv/mmu.c b/arch/x86/hyperv/mmu.c
index bd13736d0c054..0ad2378fe6ad7 100644
--- a/arch/x86/hyperv/mmu.c
+++ b/arch/x86/hyperv/mmu.c
@@ -68,15 +68,6 @@ static void hyperv_flush_tlb_multi(const struct cpumask *cpus,
 
 	local_irq_save(flags);
 
-	/*
-	 * Only check the mask _after_ interrupt has been disabled to avoid the
-	 * mask changing under our feet.
-	 */
-	if (cpumask_empty(cpus)) {
-		local_irq_restore(flags);
-		return;
-	}
-
 	flush_pcpu = (struct hv_tlb_flush **)
 		     this_cpu_ptr(hyperv_pcpu_input_arg);
 
@@ -115,7 +106,9 @@ static void hyperv_flush_tlb_multi(const struct cpumask *cpus,
 		 * must. We will also check all VP numbers when walking the
 		 * supplied CPU set to remain correct in all cases.
 		 */
-		if (hv_cpu_number_to_vp_number(cpumask_last(cpus)) >= 64)
+		cpu = cpumask_last(cpus);
+
+		if (cpu < nr_cpumask_bits && hv_cpu_number_to_vp_number(cpu) >= 64)
 			goto do_ex_hypercall;
 
 		for_each_cpu(cpu, cpus) {
@@ -131,6 +124,12 @@ static void hyperv_flush_tlb_multi(const struct cpumask *cpus,
 			__set_bit(vcpu, (unsigned long *)
 				  &flush->processor_mask);
 		}
+
+		/* nothing to flush if 'processor_mask' ends up being empty */
+		if (!flush->processor_mask) {
+			local_irq_restore(flags);
+			return;
+		}
 	}
 
 	/*



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux