Re: [PATCH 9/9] locking/lockdep: Migrate and compact boot-time dependency graph from __initdata

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