在 2026/9/11 17:51, Ye Liu 写道:
>
>
> 在 2026/9/11 16:13, [email protected] 写道:
>> 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.
>> --
>>
> Unrelated to this patch. I will fix it later.
>
>> 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:
This appears to be a false positive.
In the memory_failure() path, the folio lock is acquired at line 2597
(folio_lock(folio)) and held through the call to hwpoison_user_mappings()
at line 2635, which in turn calls collect_procs() at line 1694. There is
no folio_unlock() between the lock acquisition and the collect_procs()
call, so the lock is held throughout.（The line numbers might not match
exactly, but they shouldn't be far off.）
Consequently, the check in collect_procs():
if (!folio->mapping)
return;
and the subsequent read in collect_procs_file():
struct address_space *mapping = folio->mapping;
are both performed under the folio lock. Since all truncation paths
that set folio->mapping = NULL require the folio lock (page_cache_delete()
and filemap_remove_folio() both assert folio_test_locked()), no concurrent
truncation can modify folio->mapping between these two reads.
No fix is needed.
>>
>> 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;
>>
>
--
Thanks,
Ye Liu
