Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] Data race and Use-After-Clear on `dt->stage_m` between `dept_request_event_wait_commit()` and `dept_ttwu_stage_wait()` - [High] Use-after-free and RCU list corruption because `dept_class` objects are freed before the RCU grace period elapses - [High] Deadlock on PREEMPT_RT due to `local_irq_disable()` being used with a raw spinlock (`dept_pool_spin`) --
commit 1fd2695b58a470d54e232dcaaf112ad8a807718e Author: Byungchul Park <[email protected]> dept: implement DEPT(DEPendency Tracker) This commit implements DEPT (Dependency Tracker) to monitor waits and events for deadlock detection. It addresses Lockdep's limitations regarding read locks and cross-event scenarios. > diff --git a/kernel/dependency/dept.c b/kernel/dependency/dept.c > new file mode 100644 > index 0000000000000..52b9669fe0179 > --- /dev/null > +++ b/kernel/dependency/dept.c > @@ -0,0 +1,2977 @@ [ ... ] > +static void dept_wq_work_fn(struct work_struct *work) > +{ > + int i; > + > + for (i = 0; i < OBJECT_NR; i++) { > + struct dept_pool *p = dept_pool + i; > + int sz = p->tot_nr * p->obj_sz; > + void *rpool; > + bool need; > + > + local_irq_disable(); > + arch_spin_lock(&dept_pool_spin); [Severity: High] Is this locking pattern safe on PREEMPT_RT? By using local_irq_disable() with a raw spinlock (dept_pool_spin), hardware interrupts aren't disabled on PREEMPT_RT. Since DEPT instruments hardirq paths via dept_enter(), could a hardware interrupt fire here, attempt to acquire the exact same lock in from_pool(), and cause a hard deadlock? Should raw_local_irq_disable() or arch_local_irq_save() be used instead? [ ... ] > + list_for_each_entry_safe(c, n, &dept_classes, all_node) { > + if (!within((void *)c->key, start, sz) && > + !within(c->name, start, sz)) > + continue; > + > + hash_del_class(c); > + disconnect_class(c); > + list_del(&c->all_node); > + invalidate_class(c); > + > + /* > + * Actual deletion will happen on the rcu callback > + * that has been added in disconnect_class(). > + */ > + del_class(c); > + } > + dept_unlock(); > + dept_exit(flags); > + > + /* > + * Wait until even lockless hash_lookup_class() for the class > + * returns NULL. > + */ > + might_sleep(); > + synchronize_rcu(); [Severity: High] Does this sequence safely handle RCU lifetimes for the dept_class objects? Although the comment states actual deletion happens in the RCU callback, disconnect_class() only queues callbacks for dept_dep objects, not the class itself. By calling del_class(c) directly before synchronize_rcu(), the object is pushed to the freelist synchronously via to_pool(). If a concurrent lockless RCU reader like hash_lookup_class() traverses the bucket and reads hash_node.next after the class is reallocated, wouldn't this result in list corruption or use-after-free crashes? [ ... ] > +void dept_request_event_wait_commit(void) > +{ > + struct dept_task *dt = dept_task(); > + unsigned long flags; > + unsigned int wg; > + unsigned long ip; > + const char *w_fn; > + bool sched_map; [ ... ] > + wg = next_wgen(); > + WRITE_ONCE(dt->stage_m.wgen, wg); > + arch_spin_unlock(&dt->stage_lock); > + > + __dept_wait(&dt->stage_m, 1UL, ip, w_fn, 0, true, sched_map); [Severity: High] Can this sequence lead to a use-after-clear of dt->stage_m? Once dt->stage_lock is dropped, a concurrent wakeup can execute dept_ttwu_stage_wait() which acquires the same stage_lock and calls __dept_clean_stage(), clearing the original stage_m using memset(). Since __dept_wait() is called here with a reference to &dt->stage_m after the lock is released, could it read memory that is actively being zeroed out, resulting in a NULL pointer dereference for fields like keys or name in check_new_class()? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
