Skip to content

Commit f6564fc

Browse files
melverakpm00
authored andcommitted
mm, kmsan: fix infinite recursion due to RCU critical section
Alexander Potapenko writes in [1]: "For every memory access in the code instrumented by KMSAN we call kmsan_get_metadata() to obtain the metadata for the memory being accessed. For virtual memory the metadata pointers are stored in the corresponding `struct page`, therefore we need to call virt_to_page() to get them. According to the comment in arch/x86/include/asm/page.h, virt_to_page(kaddr) returns a valid pointer iff virt_addr_valid(kaddr) is true, so KMSAN needs to call virt_addr_valid() as well. To avoid recursion, kmsan_get_metadata() must not call instrumented code, therefore ./arch/x86/include/asm/kmsan.h forks parts of arch/x86/mm/physaddr.c to check whether a virtual address is valid or not. But the introduction of rcu_read_lock() to pfn_valid() added instrumented RCU API calls to virt_to_page_or_null(), which is called by kmsan_get_metadata(), so there is an infinite recursion now. I do not think it is correct to stop that recursion by doing kmsan_enter_runtime()/kmsan_exit_runtime() in kmsan_get_metadata(): that would prevent instrumented functions called from within the runtime from tracking the shadow values, which might introduce false positives." Fix the issue by switching pfn_valid() to the _sched() variant of rcu_read_lock/unlock(), which does not require calling into RCU. Given the critical section in pfn_valid() is very small, this is a reasonable trade-off (with preemptible RCU). KMSAN further needs to be careful to suppress calls into the scheduler, which would be another source of recursion. This can be done by wrapping the call to pfn_valid() into preempt_disable/enable_no_resched(). The downside is that this sacrifices breaking scheduling guarantees; however, a kernel compiled with KMSAN has already given up any performance guarantees due to being heavily instrumented. Note, KMSAN code already disables tracing via Makefile, and since mmzone.h is included, it is not necessary to use the notrace variant, which is generally preferred in all other cases. Link: https://lkml.kernel.org/r/[email protected] [1] Link: https://lkml.kernel.org/r/[email protected] Fixes: 5ec8e8e ("mm/sparsemem: fix race in accessing memory_section->usage") Signed-off-by: Marco Elver <[email protected]> Reported-by: Alexander Potapenko <[email protected]> Reported-by: [email protected] Reviewed-by: Alexander Potapenko <[email protected]> Tested-by: Alexander Potapenko <[email protected]> Cc: Charan Teja Kalla <[email protected]> Cc: Borislav Petkov (AMD) <[email protected]> Cc: Dave Hansen <[email protected]> Cc: Dmitry Vyukov <[email protected]> Cc: "H. Peter Anvin" <[email protected]> Cc: Ingo Molnar <[email protected]> Cc: Thomas Gleixner <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
1 parent 9319b64 commit f6564fc

File tree

2 files changed

+19
-4
lines changed

2 files changed

+19
-4
lines changed

arch/x86/include/asm/kmsan.h

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ static inline bool kmsan_virt_addr_valid(void *addr)
6464
{
6565
unsigned long x = (unsigned long)addr;
6666
unsigned long y = x - __START_KERNEL_map;
67+
bool ret;
6768

6869
/* use the carry flag to determine if x was < __START_KERNEL_map */
6970
if (unlikely(x > y)) {
@@ -79,7 +80,21 @@ static inline bool kmsan_virt_addr_valid(void *addr)
7980
return false;
8081
}
8182

82-
return pfn_valid(x >> PAGE_SHIFT);
83+
/*
84+
* pfn_valid() relies on RCU, and may call into the scheduler on exiting
85+
* the critical section. However, this would result in recursion with
86+
* KMSAN. Therefore, disable preemption here, and re-enable preemption
87+
* below while suppressing reschedules to avoid recursion.
88+
*
89+
* Note, this sacrifices occasionally breaking scheduling guarantees.
90+
* Although, a kernel compiled with KMSAN has already given up on any
91+
* performance guarantees due to being heavily instrumented.
92+
*/
93+
preempt_disable();
94+
ret = pfn_valid(x >> PAGE_SHIFT);
95+
preempt_enable_no_resched();
96+
97+
return ret;
8398
}
8499

85100
#endif /* !MODULE */

include/linux/mmzone.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2013,17 +2013,17 @@ static inline int pfn_valid(unsigned long pfn)
20132013
if (pfn_to_section_nr(pfn) >= NR_MEM_SECTIONS)
20142014
return 0;
20152015
ms = __pfn_to_section(pfn);
2016-
rcu_read_lock();
2016+
rcu_read_lock_sched();
20172017
if (!valid_section(ms)) {
2018-
rcu_read_unlock();
2018+
rcu_read_unlock_sched();
20192019
return 0;
20202020
}
20212021
/*
20222022
* Traditionally early sections always returned pfn_valid() for
20232023
* the entire section-sized span.
20242024
*/
20252025
ret = early_section(ms) || pfn_section_valid(ms, pfn);
2026-
rcu_read_unlock();
2026+
rcu_read_unlock_sched();
20272027

20282028
return ret;
20292029
}

0 commit comments

Comments
 (0)