Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] The `release_module_tags()` function modifies the `mod_area_mt` maple tree while holding `mas_lock` (a spinlock), which forces `mas_store()` to use `GFP_NOWAIT` for internal node allocations. If the tree needs to allocate a node (e.g. to merge adjacent `NULL` slots via `mas_wr_extend_null()`) and memory pressure causes the allocation to fail, `mas_store()` silently aborts and leaves the maple state in an error condition. Because `release_module_tags()` does not check `mas_is_err(&mas)` or preallocate nodes, the `mod` pointer remains in the tree after the module is freed. If a subsequently loaded module reuses the exact same `struct module *` address and then is unloaded, `release_module_tags()` will match the stale pointer. This causes it to process the stale tag area, leading to a Use-After-Free when `pr_info` dereferences the old module's freed `.rodata` via `tag->ct.modname`. -- commit 85214c8b1888956f945e1850b83b26c40f80620f Author: Hao Ge <[email protected]> alloc_tag: move release_module_tags() above reserve_module_tags() This patch moves the release_module_tags() helper function above reserve_module_tags() in the file. This allows reserve_module_tags() to call it for cleanup on failure paths without needing a forward declaration. Cc: [email protected] > diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c > index b334103104779..2070e682fe106 100644 > --- a/mm/alloc_tag.c > +++ b/mm/alloc_tag.c [ ... ] > @@ -835,6 +835,52 @@ static int vm_module_tags_populate(void) > return 0; > } > > +static void release_module_tags(struct module *mod, bool used) > +{ > + MA_STATE(mas, &mod_area_mt, module_tags.size, module_tags.size); [ ... ] > +release_area: > + mas_store(&mas, used ? &unloaded_mod : NULL); > + val = mas_prev_range(&mas, 0); > + if (val == &prepend_mod) > + mas_store(&mas, NULL); [Severity: High] This isn't a bug introduced by this patch, but since the code is being moved here, could these mas_store() calls fail silently under memory pressure? Since release_module_tags() modifies the maple tree while holding mas_lock() (a spinlock), mas_store() uses GFP_NOWAIT for internal node allocations, such as when it needs to merge adjacent NULL slots. If a GFP_NOWAIT allocation fails, mas_store() aborts and leaves the maple state in an error condition. Because this function doesn't preallocate nodes before acquiring the lock or check mas_is_err(&mas) after the store, the stale module pointer remains in the tree. If a subsequently loaded module reuses the exact same struct module address and is later unloaded, could this cause release_module_tags() to process the stale entry instead? This appears it would trigger a Use-After-Free when pr_info() dereferences the old module's freed .rodata section via tag->ct.modname. > +out: > + mas_unlock(&mas); > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
