Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Potential NULL pointer dereference or Use-After-Free of `folio->mapping` in `collect_procs_file()` due to concurrent truncation. -- commit 788d1f422c3f19026d9e0d86b550c238607720ed Author: Ye Liu <[email protected]> mm/memory-failure: convert process iterator to for_each_process_rculock This commit replaces the manual rcu_read_lock()/rcu_read_unlock() pairs combined with for_each_process() loop in mm/memory-failure.c with for_each_process_rculock(), which scopes the RCU read lock to the loop body via scoped_guard(rcu). > diff --git a/mm/memory-failure.c b/mm/memory-failure.c > index a8b03e2920ba8..7d93deb1f7b3d 100644 > --- a/mm/memory-failure.c > +++ b/mm/memory-failure.c [ ... ] > @@ -589,9 +587,8 @@ static void collect_procs_file(const struct folio *folio, > pgoff_t pgoff; > > i_mmap_lock_read(mapping); [Severity: High] This is a pre-existing issue, but could this lead to a NULL pointer dereference or a use-after-free of mapping due to a race with truncation? In memory_failure(), the folio lock is dropped in hwpoison_user_mappings() before calling collect_procs(). collect_procs() performs an unlocked check of !folio->mapping: static void collect_procs(...) { if (!folio->mapping) return; ... else collect_procs_file(folio, page, tokill, force_early); } If a concurrent truncation removes the folio from the page cache immediately after this check, folio->mapping becomes NULL. collect_procs_file() then initializes mapping = folio->mapping and calls i_mmap_lock_read(mapping). If the compiler performs a second read of folio->mapping, mapping will be NULL, crashing the kernel on the semaphore acquisition. If the compiler caches the first read, mapping will point to an address_space whose host inode may have been concurrently freed, leading to a use-after-free since the elevated page refcount does not pin the inode and no RCU read lock is held during the blocking i_mmap_lock_read() call. > - rcu_read_lock(); > pgoff = page_pgoff(folio, page); > - for_each_process(tsk) { > + for_each_process_rculock(tsk) { > struct task_struct *t = task_early_kill(tsk, force_early); > unsigned long addr; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
