Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [High] Invalid Wait Context on PREEMPT_RT and potential buddy allocator recursion. - [High] Use-After-Free triggered by reading /proc/lockdep after boot compaction failure. - [High] Permanent memory leak of dynamically allocated lock graph edges on module unload. - [Medium] Integer underflow in global active edge counter `nr_list_entries`. --
--- Patch [9]: [PATCH 9/9] locking/lockdep: Migrate and compact boot-time dependency graph from __initdata --- commit f7e00f7b01be3f305f806e2c70e4d05c87137496 Author: Jim Cromie <[email protected]> locking/lockdep: Migrate and compact boot-time dependency graph from __initdata This commit moves early boot lockdep nodes to __initdata and compacts them into folio_pool allocations during late_initcall to save memory. > diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c > index 5968a976bf8ea..d9541207b342b 100644 > --- a/kernel/locking/lockdep.c > +++ b/kernel/locking/lockdep.c [ ... ] > @@ -6295,30 +6308,26 @@ static void zap_class(struct pending_free *pf, struct > lock_class *class) > list_for_each_entry_safe(entry, tmp, &class->locks_after, entry) { > list_for_each_entry_safe(other, other_tmp, > &entry->links_to->locks_before, entry) { > if (other->links_to == class) { > - if (is_bootstrap_entry(other)) > - __clear_bit(other - list_entries, > list_entries_in_use); > + clear_bootstrap_entry_bit(other); > nr_list_entries--; > list_del_rcu(&other->entry); [Severity: High] Does this leak the dynamically allocated edges? When lock classes are dynamically destroyed during module unload, zap_class() unlinks their graph edges using list_del_rcu(). With folio_pool allocations, there is no explicit kfree() or folio_pool_free_obj() called. Discarding the pointers leaves the memory stranded, leading to a permanent memory leak until the bump allocator is exhausted. [ ... ] > @@ -6733,12 +6745,101 @@ void __init lockdep_init(void) > static int __init lockdep_boot_report(void) [ ... ] > +static int __init lockdep_compact_boot_graph(void) > +{ > + struct lock_class *class; > + struct lock_list *entry, *tmp, *new_entry; > + unsigned long flags; > + unsigned long migrated = 0; [ ... ] > + raw_local_irq_save(flags); > + if (!graph_lock()) { > + raw_local_irq_restore(flags); > + return 0; > + } > + > + list_for_each_entry(class, &all_lock_classes, lock_entry) { > + list_for_each_entry_safe(entry, tmp, &class->locks_after, > entry) { > + if (is_bootstrap_entry(entry)) { > + if (new_entry) { > + *new_entry = *entry; > + list_replace_rcu(&entry->entry, > &new_entry->entry); > + new_entry = NULL; > + } else { > + struct lock_list *slot; > + > + slot = > folio_pool_alloc_type(&lockdep_pool, > + struct > lock_list, > + > GFP_ATOMIC); [Severity: High] Can this cause an invalid wait context panic on PREEMPT_RT? The folio_pool_alloc_type() function is called here while holding graph_lock(). Because graph_lock() acquires a raw spinlock with local IRQs disabled, it cannot safely call into folio_pool_alloc_type(). Internally, folio_pool_alloc_type() acquires a standard spinlock_t, which maps to a sleepable rt_mutex on PREEMPT_RT. Acquiring a sleeping lock inside a raw spinlock context violates the wait context rules. Additionally, if the pre-allocated chunk is exhausted, this could call folio_alloc(), which takes sleepable MM zone locks and triggers the exact recursion the pre-allocation was intended to avoid. > + if (!slot) { > + debug_locks_off_graph_unlock(); > + raw_local_irq_restore(flags); > + pr_err("lockdep: folio chunk > exhausted during boot compaction\n"); > + return -ENOMEM; [Severity: High] Could this lead to a use-after-free when reading /proc/lockdep? If folio_pool_alloc_type() fails, this error path disables lockdep and aborts. However, the partially migrated dependency graph still contains pointers to early_list_entries, which resides in the __initdata section. When free_initmem() later reclaims the __initdata section, these pointers become dangling. If /proc/lockdep is read, l_show() unconditionally traverses the class->locks_after list without checking if debug_locks is enabled, which would dereference the freed init memory. [ ... ] > + } > + } > + > + /* Adjust counter so compaction does not double-count migrated nodes */ > + nr_list_entries -= migrated; [Severity: Medium] Is this subtraction correct? The compaction loop replaces boot nodes with new allocations directly from folio_pool without incrementing nr_list_entries. Subtracting migrated from nr_list_entries here reduces the counter even though the active edge count remains unchanged. When these dynamically allocated lock classes are later freed during module unload, zap_class() unconditionally decrements nr_list_entries--, which will cause the global counter to underflow. > + > + bootstrap_entries = NULL; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=9
