Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] Atomic context violation and RT-deadlock in lockdep dynamic allocation. - [High] Use-After-Free and memory leak due to untracked dynamic lock dependencies. - [Low] Undefined behavior in pointer arithmetic for dynamically allocated lock entries. --
--- Patch [6]: [PATCH 6/9] locking/lockdep: Fallback to folio_pool in alloc_list_entry when static pool is full --- commit 618b3b19dc7c789afe3ea0f17bf4c674f06ef1f3 Author: Jim Cromie <[email protected]> locking/lockdep: Fallback to folio_pool in alloc_list_entry when static pool is full Use struct folio_pool to dynamically expand lock dependency storage when the static list_entries pool is exhausted, avoiding premature lockdep disabling. > diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c > index 2d4c5bab5af88..f0f58db090fff 100644 > --- a/kernel/locking/lockdep.c > +++ b/kernel/locking/lockdep.c [ ... ] > @@ -1404,11 +1429,19 @@ static struct lock_list *alloc_list_entry(void) > ARRAY_SIZE(list_entries)); > > if (idx >= ARRAY_SIZE(list_entries)) { > + struct lock_list *p; > + > + p = folio_pool_alloc_type(&lockdep_pool, struct lock_list, > + GFP_ATOMIC); [Severity: High] Does dynamically allocating via folio_pool while holding a raw spinlock violate atomic context rules? alloc_list_entry() is called by add_lock_to_list() during lockdep graph updates, which always run under the raw lockdep_lock (via graph_lock()). Even with GFP_ATOMIC, folio_pool_alloc_type() acquires a standard spinlock_t (sp->lock) and can call into the page allocator. On PREEMPT_RT, acquiring a sleepable spinlock_t or calling the page allocator from within a raw spinlock section causes a kernel panic (sleeping in atomic context). On non-RT kernels, calling the allocator inside lockdep's graph lock violates its strict non-reentrancy rules, which risks deadlocks if the allocator triggers warnings or console output. > + if (p) { > + nr_list_entries++; > + return p; > + } [Severity: High] Could these dynamically allocated lock entries cause a use-after-free and memory leak when modules are unloaded? Dynamically allocated dependencies are not tracked in the list_entries_in_use bitmap. When a module is unloaded, zap_class() unlinks dependencies: kernel/locking/lockdep.c:zap_class() { ... for_each_set_bit(i, list_entries_in_use, ARRAY_SIZE(list_entries)) { entry = list_entries + i; if (entry->class != class && entry->links_to != class) continue; __clear_bit(i, list_entries_in_use); nr_list_entries--; list_del_rcu(&entry->entry); } ... } Because dynamic entries aren't in this bitmap, zap_class() will skip them and fail to unlink them from class->locks_after and class->locks_before. When the module's memory is freed, lockdep retains dangling pointers to the module's lock structures. Furthermore, the folio_pool bump allocator does not support freeing individual elements, making the leaked dependencies permanently unrecoverable. [Severity: Low] Could this result in undefined behavior when logging lock errors? When lockdep detects a structural mismatch, it prints an error using pointer subtraction: kernel/locking/lockdep.c:class_lock_list_valid() { ... list_for_each_entry(e, h, entry) { if (e->links_to != c) { printk(KERN_INFO "class %s: mismatch for lock entry %ld; class %s <> %s", c->name ? : "(?)", (unsigned long)(e - list_entries), ... } If the entry e was dynamically allocated via folio_pool, it does not reside within the static list_entries array. Subtracting pointers from different memory objects is undefined behavior and produces a nonsensical array index in the log. > if (!debug_locks_off_graph_unlock()) > return NULL; > > nbcon_cpu_emergency_enter(); > - print_lockdep_off("BUG: MAX_LOCKDEP_ENTRIES too low!"); > + print_lockdep_off("BUG: MAX_LOCKDEP_ENTRIES too low and > folio_pool exhausted!"); > dump_stack(); > nbcon_cpu_emergency_exit(); > return NULL; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
