Re: [RFC PATCH 12/18] mm/filemap: don't defer the free of folios of an inode being evicted

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] mm/filemap: premature PG_rcu_free clear on live inodes with AS_EXITING
--

commit 47ca48a16967ca2b270af7537211abb87d17221b
Author: Kiryl Shutsemau (Meta) <kas@kernel.org>

mm/filemap: don't defer the free of folios of an inode being evicted

This commit clears PG_rcu_free on folios in page_cache_delete() and
page_cache_delete_batch() when mapping_exiting() is set, avoiding
RCU-deferred freeing during inode eviction.

> diff --git a/mm/filemap.c b/mm/filemap.c
> index b732ecaa78a90..53ec922799771 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -138,6 +138,9 @@ static void page_cache_delete(struct address_space *mapping,
>  	nr = folio_nr_pages(folio);
>  
>  	VM_BUG_ON_FOLIO(!folio_test_locked(folio), folio);
> +	/* No lockless reader can be in an evicted inode */
> +	if (folio_test_rcu_free(folio) && mapping_exiting(mapping))
> +		folio_clear_rcu_free(folio);
>  
>  	xas_store(&xas, shadow);
>  	xas_init_marks(&xas);
> @@ -306,6 +309,9 @@ static void page_cache_delete_batch(struct address_space *mapping,
>  		}
>  
>  		WARN_ON_ONCE(!folio_test_locked(folio));
> +		/* No lockless reader can be in an evicted inode */
> +		if (folio_test_rcu_free(folio) && mapping_exiting(mapping))
> +			folio_clear_rcu_free(folio);

[Severity: Critical]
Can clearing PG_rcu_free when mapping_exiting(mapping) is set cause a
use-after-free if truncate_inode_pages_final() is called on a live, open
file?

In mm/truncate.c, truncate_inode_pages_final() unconditionally sets
AS_EXITING via mapping_set_exiting():

mm/truncate.c:truncate_inode_pages_final() {
	mapping_set_exiting(mapping);

	if (!mapping_empty(mapping)) {
		xa_lock_irq(&mapping->i_pages);
		xa_unlock_irq(&mapping->i_pages);
	}

	truncate_inode_pages(mapping, 0);
	...
}

However, f2fs calls truncate_inode_pages_final() on live, open regular
files during atomic write operations, such as when starting an atomic
replace via ioctl(F2FS_IOC_START_ATOMIC_REPLACE):

fs/f2fs/file.c:f2fs_ioc_start_atomic_write() {
	...
	if (truncate) {
		set_inode_flag(inode, FI_ATOMIC_REPLACE);
		truncate_inode_pages_final(inode->i_mapping);
		f2fs_i_size_write(inode, 0);
		isize = 0;
	}
	...
}

and when aborting an atomic write:

fs/f2fs/segment.c:f2fs_abort_atomic_write() {
	...
	if (clean)
		truncate_inode_pages_final(inode->i_mapping);
	...
}

Buffered reads in f2fs call f2fs_file_read_iter() -> filemap_read()
without holding inode_lock() or fi->i_gc_rwsem, which marks folios with
PG_rcu_free via filemap_mark_hot() and copies from them locklessly under
rcu_read_lock().

Furthermore, because AS_EXITING is never cleared once set,
mapping_exiting(inode->i_mapping) remains true for the rest of the live
inode's lifetime in cache. During both the atomic-write
truncate_inode_pages_final() calls and any subsequent truncate(), hole
punch, fadvise(POSIX_FADV_DONTNEED) invalidation, or page reclaim on that
open file, page_cache_delete() and page_cache_delete_batch() will clear
PG_rcu_free on hot folios.

When the last reference to those folios is dropped, wouldn't
__free_pages_prepare() skip rcu_free_defer() and return the pages
immediately to the buddy allocator while concurrent lockless readers in
filemap_read() are still copying from them under rcu_read_lock()?

>  
>  		folio->mapping = NULL;
>  		/* Leave folio->index set: truncation lookup relies on it */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019154913.1765974-1-kas@kernel.org?part=12