Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Lockless traversal of `dept_classes` and `fc->dep_head` in procfs read operations causes kernel panic due to LIST_POISON pointer dereference and Use-After-Free. --
commit e2e753e7050a1aa0365bc001321fe4e79158b5b4 Author: Byungchul Park <[email protected]> dept: add proc knobs to show stats and dependency graph This commit introduces procfs knobs to expose internal stats and the dependency graph for the dependency tracker at runtime. > diff --git a/kernel/dependency/dept_proc.c b/kernel/dependency/dept_proc.c > new file mode 100644 > index 0000000000000..97beaf397715f > --- /dev/null > +++ b/kernel/dependency/dept_proc.c [ ... ] > +static void *l_next(struct seq_file *m, void *v, loff_t *pos) > +{ > + /* > + * XXX: Serialize list traversal if needed. The following might > + * give a wrong information on contention. > + */ > + return seq_list_next(v, &dept_classes, pos); > +} > + > +static void *l_start(struct seq_file *m, loff_t *pos) > +{ > + /* > + * XXX: Serialize list traversal if needed. The following might > + * give a wrong information on contention. > + */ > + return seq_list_start_head(&dept_classes, *pos); > +} [Severity: High] Is it safe to traverse dept_classes without locks here? If a concurrent module unload occurs, dept_free_range() calls list_del() on the class node: kernel/dependency/dept.c:dept_free_range() { ... list_del(&c->all_node); ... } Because list_del() poisons the removed node's next pointer with LIST_POISON1, a concurrent lockless reader in seq_list_next() will dereference LIST_POISON1 and cause a kernel panic, rather than just returning wrong information. > + > +static void l_stop(struct seq_file *m, void *v) > +{ > +} > + > +static int l_show(struct seq_file *m, void *v) > +{ > + struct dept_class *fc = list_entry(v, struct dept_class, all_node); > + struct dept_dep *d; > + const char *prefix; > + > + if (v == &dept_classes) { > + seq_puts(m, "All classes:\n\n"); > + return 0; > + } > + > + prefix = fc->sched_map ? "<sched> " : ""; > + seq_printf(m, "[%p] %s%s\n", (void *)fc->key, prefix, fc->name); > + > + /* > + * XXX: Serialize list traversal if needed. The following might > + * give a wrong information on contention. > + */ > + list_for_each_entry(d, &fc->dep_head, dep_node) { [Severity: High] Does traversing fc->dep_head without rcu_read_lock() risk a use-after-free? Since the dependency objects in this list are RCU-managed, traversing them without holding rcu_read_lock() might allow the objects to be freed to the pool while they are still being iterated. > + struct dept_class *tc = d->wait->class; > + > + prefix = tc->sched_map ? "<sched> " : ""; > + seq_printf(m, " -> [%p] %s%s\n", (void *)tc->key, prefix, > tc->name); > + } > + seq_puts(m, "\n"); > + > + return 0; > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
