Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] TOCTOU race in sysctl handler allows memory profiling to remain active without support structures. -- --- Patch [3]: [RFC PATCH 3/3] alloc_tag: remove /proc/allocinfo outside of mod_lock --- commit 8d4c5cdeb441b4a47f3511f205a68485820706b0 Author: Hao Ge <[email protected]> alloc_tag: remove /proc/allocinfo outside of mod_lock shutdown_mem_profiling() calls remove_proc_entry() from reserve_module_tags(), which runs under mod_lock held for write. remove_proc_entry() waits for readers, and a reader takes mod_lock for read in allocinfo_start(). Move remove_proc_entry() to a workqueue. > diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c > index 26c6ebc96a6ae..65d0ae9af94b2 100644 > --- a/mm/alloc_tag.c > +++ b/mm/alloc_tag.c [ ... ] > @@ -600,7 +607,7 @@ static void shutdown_mem_profiling(bool remove_file) > return; > > if (remove_file) > - remove_proc_entry(ALLOCINFO_FILE_NAME, NULL); > + schedule_work(&remove_allocinfo_work); > mem_profiling_support = false; > } [Severity: High] This isn't a bug introduced by this patch, but does shutdown_mem_profiling() have a TOCTOU race with the sysctl handler proc_mem_profiling_handler()? If a concurrent module load fails setup and calls shutdown_mem_profiling(), it disables the static key and sets mem_profiling_support = false. Concurrently, could a sysctl write to /proc/sys/vm/mem_profiling evaluate mem_profiling_support before it is set to false: mm/alloc_tag.c:proc_mem_profiling_handler() { ... if (!mem_profiling_support) return -EINVAL; } return proc_do_static_key(table, write, buffer, lenp, ppos); } and then execute proc_do_static_key() after shutdown_mem_profiling() has finished, errantly re-enabling the static key? If the static key is enabled but mem_profiling_support is false, any subsequent module load will skip allocating per-cpu tag counters. When the new module executes a memory allocation, would alloc_tag_ref_set() unconditionally dereference a NULL tag->counters? include/linux/alloc_tag.h:alloc_tag_ref_set() { ... this_cpu_inc(tag->counters->calls); ... } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
