Re: [PATCH] mm/khugepaged: reject shmem collapse ranges beyond i_size

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Medium] mm: xarray node memory leak in collapse_file() [1]
--

commit 8b05158695205f614e5dcb777a5926b63afd5388
Author: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>

mm/khugepaged: reject shmem collapse ranges beyond i_size

This commit holds the inode i_rwsem in shared mode during shmem collapse in
collapse_file() and rejects ranges extending beyond i_size rounded up to
PAGE_SIZE.

> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index c6d5ba6c88a6d..e0e602e8f87f1 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -2272,6 +2272,13 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
>  	 */
>  	VM_WARN_ON_ONCE(!is_shmem && !mapping_pmd_folio_support(mapping));
>  	VM_WARN_ON_ONCE(start & (HPAGE_PMD_NR - 1));
> +	if (is_shmem)
> +		inode_lock_shared(mapping->host);
> +	if (is_shmem &&
> +	    end > DIV_ROUND_UP(i_size_read(mapping->host), PAGE_SIZE)) {
> +		result = SCAN_TRUNCATED;
> +		goto out;
> +	}
>  
>  	result = alloc_charge_folio(&new_folio, mm, cc, HPAGE_PMD_ORDER);
>  	if (result != SCAN_SUCCEED)
> @@ -2703,6 +2710,8 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
>  out_put:
>  	folio_put(new_folio);
>  out:
> +	if (is_shmem)
> +		inode_unlock_shared(mapping->host);
>  	VM_BUG_ON(!list_empty(&pagelist));
>  	trace_mm_khugepaged_collapse_file(mm, new_pfn, index, addr, is_shmem, file, HPAGE_PMD_NR, result);
>  	return result;

[Severity: Medium]
This problem wasn't introduced by this patch, but can collapse_file() leak
a preallocated xa_node in xas.xa_alloc [1]?

mm/khugepaged.c:collapse_file() {
	...
	do {
		xas_lock_irq(&xas);
		xas_create_range(&xas);
		if (!xas_error(&xas))
			break;
		xas_unlock_irq(&xas);
		if (!xas_nomem(&xas, GFP_KERNEL)) {
			result = SCAN_FAIL;
			goto rollback;
		}
	} while (1);
	...
}

If xas_create_range() fails with -ENOMEM, xas_nomem() drops the lock and
caches a newly allocated xa_node in xas.xa_alloc.  If another thread
populates the XArray range before the lock is retaken, the next
xas_create_range() call succeeds without consuming xas.xa_alloc and breaks
out of the loop.  Because collapse_file() never calls xas_destroy(&xas)
before returning, that xa_node is leaked.

[1] https://sashiko.dev/#/bug/linux-2a262b48-1e7f-4d11-8b87-d5e410ce41f7

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112142335.2749597-2-Jeremy.Jean@oss.cyber.gouv.fr?part=1