On Tue, Jul 07, 2026 at 07:33:22AM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 5 potential issue(s)
> to consider:
> - [Critical] dept_key_destroy() synchronously recycles dept_class objects to
> the lockless pool before the RCU grace period completes, causing UAF and list
> traversal corruption for concurrent lockless readers.
> - [High] rwsem_acquire_read and rwsem_acquire incorrectly use ldt_lock(),
> breaking reader-writer dependency tracking and causing false-positive
> deadlocks.
> - [High] ldt_init() is called before the `key` parameter is validated,
> causing DEPT to dereference a garbage pointer if `key` is NULL.
> - [Medium] dept_key_init() and dept_key_destroy() are called on static keys
> before validation, erroneously disabling DEPT or corrupting tracking state.
> - [Low] STATIC_LOCKDEP_MAP_INIT creates double-quoted strings for map names
> due to `#n` stringification in DEPT_MAP_INITIALIZER.
> --
>
> commit 6c1a95ee3182a2a82a1d61196d75acd387d7f3e3
> Author: Byungchul Park <[email protected]>
>
> dept: tie to lockdep and IRQ tracing
>
> This patch integrates DEPT (Dependency Tracker) into the lockdep framework
> and IRQ tracing paths. It modifies various locking primitives and lockdep
> interfaces to initialize and track DEPT maps alongside lockdep maps.
>
> > diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
> > index 621566345406d..60b17a2ff555f 100644
> > --- a/include/linux/lockdep.h
> > +++ b/include/linux/lockdep.h
> [ ... ]
> > @@ -428,7 +431,8 @@ enum xhlock_context_t {
> > * Note that _name must not be NULL.
> > */
> > #define STATIC_LOCKDEP_MAP_INIT(_name, _key) \
> > - { .name = (_name), .key = (void *)(_key), }
> > + { .name = (_name), .key = (void *)(_key), \
> > + .dmap = DEPT_MAP_INITIALIZER(_name, _key) }
>
> [Severity: Low]
> Since _name is usually passed as a string literal to
> STATIC_LOCKDEP_MAP_INIT, does passing it to DEPT_MAP_INITIALIZER, which
> uses the stringification operator on its first argument, result in
> double-quoted strings for map names in DEPT's debug output?
I will check it. Thanks.
> [ ... ]
> > -#define rwsem_acquire(l, s, t, i) lock_acquire_exclusive(l, s,
> > t, NULL, i)
> > -#define rwsem_acquire_nest(l, s, t, n, i) lock_acquire_exclusive(l, s,
> > t, n, i)
> > -#define rwsem_acquire_read(l, s, t, i)
> > lock_acquire_shared(l, s, t, NULL, i)
> > -#define rwsem_release(l, i) lock_release(l, i)
> [ ... ]
> > +#define rwsem_acquire(l, s, t, i) \
> > +do { \
> > + ldt_lock(&(l)->dmap, s, t, NULL, i); \
> > + lock_acquire_exclusive(l, s, t, NULL, i); \
> > +} while (0)
> > +#define rwsem_acquire_nest(l, s, t, n, i) \
> > +do { \
> > + ldt_lock(&(l)->dmap, s, t, n, i); \
> > + lock_acquire_exclusive(l, s, t, n, i); \
> > +} while (0)
> > +#define rwsem_acquire_read(l, s, t, i)
> > \
> > +do { \
> > + ldt_lock(&(l)->dmap, s, t, NULL, i); \
> > + lock_acquire_shared(l, s, t, NULL, i); \
> > +} while (0)
>
> [Severity: High]
> Is it intentional that rwsem_acquire_read() uses ldt_lock() instead of
> ldt_rlock(), and rwsem_acquire() uses ldt_lock() instead of ldt_wlock()?
It's intended. rwsem is a queued lock for fairness. So it should be
considered as a normal lock to detect a potential deadlock.
> This appears to mistakenly model a shared lock as an exclusive lock. Does
> this cause DEPT to incorrectly model concurrent read locks as mutually
> exclusive, leading to false-positive circular dependency (deadlock) reports
> when concurrent readers take an rwsem and interact with other locks?
As I told you, rwsem is a queued lock. So this is not true.
> > diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> > index 2d4c5bab5af88..dc97f2753ef81 100644
> > --- a/kernel/locking/lockdep.c
> > +++ b/kernel/locking/lockdep.c
> > @@ -1224,6 +1224,8 @@ void lockdep_register_key(struct lock_class_key *key)
> > struct hlist_head *hash_head;
> > struct lock_class_key *k;
> > unsigned long flags;
> >
> > + dept_key_init(&key->dkey);
> > +
> > if (WARN_ON_ONCE(static_obj(key)))
> > return;
>
> [Severity: Medium]
> In lockdep_register_key(), dept_key_init() is called before the
> static_obj() check. If a static key is improperly passed, lockdep gracefully
> rejects it by warning and returning early.
>
> However, processing a static key in dept_key_init() triggers a DEPT_STOP
> abort. Should the DEPT initialization occur after validating the key?
I will check it.
> [ ... ]
> > @@ -4940,6 +4950,8 @@ void lockdep_init_map_type(struct lockdep_map *lock,
> > const char *name,
> > {
> > int i;
> >
> > + ldt_init(&lock->dmap, &key->dkey, subclass, name);
> > +
> > for (i = 0; i < NR_LOCKDEP_CACHING_CLASSES; i++)
> > lock->class_cache[i] = NULL;
>
> [Severity: High]
> In lockdep_init_map_type(), ldt_init() is called before the parameter
> validation checks for a NULL key further down in the function.
>
> If key is NULL, &key->dkey yields a non-NULL offset pointer. Does DEPT accept
> this garbage pointer in valid_key(), causing a kernel panic when
> check_new_class() dereferences it?
I will check it.
> [ ... ]
> > @@ -6588,6 +6608,8 @@ void lockdep_unregister_key(struct lock_class_key
> > *key)
> > bool found = false;
> > bool need_callback = false;
> >
> > + dept_key_destroy(&key->dkey);
> > +
> > might_sleep();
> >
> > if (WARN_ON_ONCE(static_obj(key)))
> > return;
>
> [Severity: Medium]
> Similarly to lockdep_register_key(), dept_key_destroy() is called here
> unconditionally before the static_obj() check. If a driver incorrectly calls
> dynamic lock destruction on a static key, it eagerly destroys its DEPT state
> while lockdep retains it. Should this also be moved after the static_obj()
> check?
Sure. I will.
> [Severity: Critical]
> Additionally, in dept_key_destroy() called here, the function removes a class
> from the RCU hash table and drops its refcount to 0 via del_class(). This
> recycles the object to the DEPT pool before synchronize_rcu() is called.
No.
del_class() never drops refcount to 0. That happens only through:
disconnect_class()->call_rcu(del_dep_rcu)
// after the grace period
del_dep_rcu()->del_dep()->put_ecxt()->put_class()
del_dep_rcu()->del_dep()->put_wait()->put_class()
So it's safe.
> Can a concurrent lockless reader in hash_lookup_class() traversing the same
> bucket via hlist_for_each_entry_rcu() access this recycled object, leading to
> a use-after-free and hash list corruption?
No.
Byungchul
>
> --
> Sashiko AI review ยท
> https://sashiko.dev/#/patchset/[email protected]?part=4
