[PATCH 2/2] x86/kmsan: Don't call instrumented code from kmsan_virt_addr_valid()

[Viorel Cernateanu via B4 Relay <devnull+vrilutza.gmail.com@kernel.org>]

From: Viorel Cernateanu <vrilutza@gmail.com>

With CONFIG_DEBUG_PREEMPT, a KMSAN kernel hangs silently right after
KMSAN is enabled during boot.

kmsan_virt_addr_valid() calls preempt_disable() and pfn_valid(), and
pfn_valid() calls rcu_read_lock_sched(). With CONFIG_DEBUG_PREEMPT both
end up in preempt_count_add(), which is instrumented. Its instrumentation
looks up metadata, which calls kmsan_virt_addr_valid() again, and so on
until the boot stack overflows.

Use a copy of pfn_valid() that disables preemption with the notrace
helpers instead. It is still an RCU-sched read-side critical section,
and it avoids the instrumented preemption and RCU/lockdep helpers on
this path.

Fixes: f6564fce256a ("mm, kmsan: fix infinite recursion due to RCU critical section")
Signed-off-by: Viorel Cernateanu <vrilutza@gmail.com>
Link: https://lore.kernel.org/20240308043448.masllzeqwht45d4j@M910t
---
 arch/x86/include/asm/kmsan.h | 49 +++++++++++++++++++++++++++++---------------
 1 file changed, 33 insertions(+), 16 deletions(-)

diff --git a/arch/x86/include/asm/kmsan.h b/arch/x86/include/asm/kmsan.h
index a71e84d6abab..17effe8be642 100644
--- a/arch/x86/include/asm/kmsan.h
+++ b/arch/x86/include/asm/kmsan.h
@@ -68,6 +68,38 @@ static inline bool kmsan_phys_addr_valid(unsigned long addr)
 		return true;
 }
 
+/*
+ * Same as pfn_valid(), but without rcu_read_lock_sched(). That one calls into
+ * instrumented code: preempt_count_add() with CONFIG_DEBUG_PREEMPT or
+ * CONFIG_TRACE_PREEMPT_TOGGLE, lock_acquire() with CONFIG_DEBUG_LOCK_ALLOC.
+ * Instrumented code looks up metadata itself, so calling it from the metadata
+ * lookup can recurse; with CONFIG_DEBUG_PREEMPT it does, until the stack
+ * overflows.
+ *
+ * Disabling preemption still makes this an RCU-sched read-side critical
+ * section. Preemption is re-enabled without a reschedule, as before, to
+ * avoid entering the scheduler from the metadata lookup.
+ */
+static inline bool kmsan_pfn_valid(unsigned long pfn)
+{
+	struct mem_section *ms;
+	bool ret;
+
+	if (PHYS_PFN(PFN_PHYS(pfn)) != pfn)
+		return false;
+
+	if (pfn_to_section_nr(pfn) >= NR_MEM_SECTIONS)
+		return false;
+	ms = __pfn_to_section(pfn);
+
+	preempt_disable_notrace();
+	ret = valid_section(ms) &&
+	      (early_section(ms) || pfn_section_valid(ms, pfn));
+	preempt_enable_no_resched_notrace();
+
+	return ret;
+}
+
 /*
  * Taken from arch/x86/mm/physaddr.c to avoid using an instrumented version.
  */
@@ -75,7 +107,6 @@ static inline bool kmsan_virt_addr_valid(void *addr)
 {
 	unsigned long x = (unsigned long)addr;
 	unsigned long y = x - __START_KERNEL_map;
-	bool ret;
 
 	/* use the carry flag to determine if x was < __START_KERNEL_map */
 	if (unlikely(x > y)) {
@@ -91,21 +122,7 @@ static inline bool kmsan_virt_addr_valid(void *addr)
 			return false;
 	}
 
-	/*
-	 * pfn_valid() relies on RCU, and may call into the scheduler on exiting
-	 * the critical section. However, this would result in recursion with
-	 * KMSAN. Therefore, disable preemption here, and re-enable preemption
-	 * below while suppressing reschedules to avoid recursion.
-	 *
-	 * Note, this sacrifices occasionally breaking scheduling guarantees.
-	 * Although, a kernel compiled with KMSAN has already given up on any
-	 * performance guarantees due to being heavily instrumented.
-	 */
-	preempt_disable();
-	ret = pfn_valid(x >> PAGE_SHIFT);
-	preempt_enable_no_resched();
-
-	return ret;
+	return kmsan_pfn_valid(x >> PAGE_SHIFT);
 }
 
 #endif /* !MODULE */

-- 
2.53.0