Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Concurrent dmem cgroup eviction can observe a partially initialized pool and trigger a NULL pointer dereference. -- commit 505dda1717b6dcbcc1903e6c7cc5e90077d70bb6 Author: linuszeng <[email protected]> Subject: mm: page_counter: track protection state in page_counter_protection This patch moves the read/write side of hierarchical protection from struct page_counter to struct page_counter_protection. It updates propagate_protected_usage and calculate_protection, along with accessors to operate on the new context structure. The patch intends to introduce no functional changes. > diff --git a/mm/page_counter.c b/mm/page_counter.c > index 38cb99f5f50ed..401201c8e3902 100644 > --- a/mm/page_counter.c > +++ b/mm/page_counter.c [ ... ] > @@ -463,23 +479,24 @@ void page_counter_calculate_protection(struct > page_counter *root, > return; > > if (parent == root) { > - counter->emin = READ_ONCE(counter->min); > - counter->elow = READ_ONCE(counter->low); > + prot->emin = READ_ONCE(prot->min); > + prot->elow = READ_ONCE(prot->low); > return; > } > > + parent_prot = parent->prot; [Severity: High] This is a pre-existing issue, but could this code encounter a null pointer dereference if concurrent dmem cgroup eviction observes a partially initialized pool? Looking at alloc_pool_single in kernel/cgroup/dmem.c, a new pool is published to the RCU-protected pools list before its parent link is fully established: alloc_pool_single() ... list_add_tail_rcu(&pool->css_node, &dmemcs->pools); ... If a concurrent thread running dmem_cgroup_state_evict_valuable or dmem_cgroup_below_min iterates over the cgroup descendants via css_for_each_descendant_pre, it can find this partially initialized pool and pass it to page_counter_calculate_protection. Since counter->parent is null and does not match the root argument, the early return above is skipped, leading to a dereference of parent->prot here when parent is null. Does the eviction path need to synchronize with the completion of pool initialization, or should page_counter_calculate_protection check if parent is null before dereferencing it? > parent_usage = page_counter_read(parent); > > - WRITE_ONCE(counter->emin, effective_protection(usage, parent_usage, > - READ_ONCE(counter->min), > - READ_ONCE(parent->emin), [ ... ] -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
