Commit 031e04bd authored by Marco Elver's avatar Marco Elver Committed by Andrew Morton
Browse files

stackdepot: fix stack_depot_save_flags() in NMI context 

Per documentation, stack_depot_save_flags() was meant to be usable from
NMI context if STACK_DEPOT_FLAG_CAN_ALLOC is unset.  However, it still
would try to take the pool_lock in an attempt to save a stack trace in the
current pool (if space is available).

This could result in deadlock if an NMI is handled while pool_lock is
already held.  To avoid deadlock, only try to take the lock in NMI context
and give up if unsuccessful.

The documentation is fixed to clearly convey this.

Link: https://lkml.kernel.org/r/Z0CcyfbPqmxJ9uJH@elver.google.com
Link: https://lkml.kernel.org/r/20241122154051.3914732-1-elver@google.com


Fixes: 4434a56e ("stackdepot: make fast paths lock-less again")
Signed-off-by: default avatarMarco Elver <elver@google.com>
Reported-by: default avatarSebastian Andrzej Siewior <bigeasy@linutronix.de>
Reviewed-by: default avatarSebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Alexander Potapenko <glider@google.com>
Cc: Andrey Konovalov <andreyknvl@gmail.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: <stable@vger.kernel.org>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
parent 6a7de1bf

include/linux/stackdepot.h

+3 −3
Original line number Diff line number Diff line
@@ -147,7 +147,7 @@ static inline int stack_depot_early_init(void) { return 0; } 
 * If the provided stack trace comes from the interrupt context, only the part

 * up to the interrupt entry is saved.

 *

 * Context: Any context, but setting STACK_DEPOT_FLAG_CAN_ALLOC is required if

 * Context: Any context, but unsetting STACK_DEPOT_FLAG_CAN_ALLOC is required if

 *          alloc_pages() cannot be used from the current context. Currently

 *          this is the case for contexts where neither %GFP_ATOMIC nor

 *          %GFP_NOWAIT can be used (NMI, raw_spin_lock).
@@ -156,7 +156,7 @@ static inline int stack_depot_early_init(void) { return 0; } 
 */

depot_stack_handle_t stack_depot_save_flags(unsigned long *entries,

					    unsigned int nr_entries,

					    gfp_t gfp_flags,

					    gfp_t alloc_flags,

					    depot_flags_t depot_flags);



/**
@@ -175,7 +175,7 @@ depot_stack_handle_t stack_depot_save_flags(unsigned long *entries, 
 * Return: Handle of the stack trace stored in depot, 0 on failure

 */

depot_stack_handle_t stack_depot_save(unsigned long *entries,

				      unsigned int nr_entries, gfp_t gfp_flags);

				      unsigned int nr_entries, gfp_t alloc_flags);



/**

 * __stack_depot_get_stack_record - Get a pointer to a stack_record struct

lib/stackdepot.c

+9 −1
Original line number Diff line number Diff line
@@ -630,7 +630,15 @@ depot_stack_handle_t stack_depot_save_flags(unsigned long *entries, 
			prealloc = page_address(page);

	}



	if (in_nmi()) {

		/* We can never allocate in NMI context. */

		WARN_ON_ONCE(can_alloc);

		/* Best effort; bail if we fail to take the lock. */

		if (!raw_spin_trylock_irqsave(&pool_lock, flags))

			goto exit;

	} else {

		raw_spin_lock_irqsave(&pool_lock, flags);

	}

	printk_deferred_enter();



	/* Try to find again, to avoid concurrently inserting duplicates. */