[PATCH] x86/mtrr: allocate the cache map before taking mtrr_mutex

[Yogesh Gaur]

mtrr_copy_map() does a GFP_KERNEL allocation with mtrr_mutex held.
It runs once at boot from mtrr_init_finalize(), but lockdep keeps
the mtrr_mutex -> fs_reclaim dependency it records there for the
life of the system. Once another path gives lockdep the rest of a
cycle back to a lock held around mtrr_mutex, the next MTRR ioctl
reports a circular dependency. syzbot found this through nbd, which
takes cpu_hotplug_lock (via sk_set_memalloc() -> static_key_slow_inc())
under its tx_lock, while mtrr_del_page() takes mtrr_mutex under
cpu_hotplug_lock:

  WARNING: possible circular locking dependency detected
  syz.9.5848/21744 is trying to acquire lock:
   (mtrr_mutex), at: mtrr_del_page arch/x86/kernel/cpu/mtrr/mtrr.c:408
  but task is already holding lock:
   (cpu_hotplug_lock), at: mtrr_del_page arch/x86/kernel/cpu/mtrr/mtrr.c:407
  -> #1 (fs_reclaim):
         fs_reclaim_acquire
         might_alloc
         slab_pre_alloc_hook
         __kmalloc_noprof
         mtrr_copy_map arch/x86/kernel/cpu/mtrr/generic.c:413
         mtrr_init_finalize arch/x86/kernel/cpu/mtrr/mtrr.c:618
  Chain exists of:
    mtrr_mutex --> &nsock->tx_lock --> cpu_hotplug_lock

The allocation does not need the mutex; only publishing the new map
does. Allocate first and take mtrr_mutex just to copy the boot-time
entries and switch cache_map over. On allocation failure cache_map is
now set to NULL explicitly rather than by the failed assignment, so
the behaviour is unchanged and cache_map still never points at the
__initdata array after init.

Fixes: 061b984aab58 ("x86/mtrr: Construct a memory map with cache modes")
Reported-by: syzbot+342762971f666337474e@xxxxxxxxxxxxxxxxxxxxxxxxx
Assisted-by: LLM
Signed-off-by: Yogesh Gaur <yogeshgaur.83@xxxxxxxxx>
---
Built with W=1 only. syzbot has no reproducer for this report, so the
fix has not been runtime-tested.

 arch/x86/kernel/cpu/mtrr/generic.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c
index 67cf69f24b00..430bc9dd6f20 100644
--- a/arch/x86/kernel/cpu/mtrr/generic.c
+++ b/arch/x86/kernel/cpu/mtrr/generic.c
@@ -402,20 +402,25 @@ void __init mtrr_build_map(void)
 void __init mtrr_copy_map(void)
 {
 	unsigned int new_size = get_cache_map_size();
+	struct cache_map *new_map;
 
 	if (!mtrr_state.enabled || !new_size) {
 		cache_map = NULL;
 		return;
 	}
 
+	/* Allocate before taking mtrr_mutex, the allocation may reclaim. */
+	new_map = kzalloc_objs(*new_map, new_size);
+
 	mutex_lock(&mtrr_mutex);
 
-	cache_map = kzalloc_objs(*cache_map, new_size);
-	if (cache_map) {
-		memmove(cache_map, init_cache_map,
-			cache_map_n * sizeof(*cache_map));
+	if (new_map) {
+		memmove(new_map, init_cache_map,
+			cache_map_n * sizeof(*new_map));
+		cache_map = new_map;
 		cache_map_size = new_size;
 	} else {
+		cache_map = NULL;
 		mtrr_state.enabled = 0;
 		pr_err("MTRRs disabled due to allocation failure for lookup map.\n");
 	}
-- 
2.55.0.windows.5