Re: [PATCH v3 04/15] mm/memory-failure: convert process iterator to for_each_process_rculock

[sashiko-bot@kernel.org]

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 <liuye@kylinos.cn>

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/20260911075800.491472-1-ye.liu@linux.dev?part=4