Re: [PATCH v5 04/19] mm, swap: use a VM_SPARSE cluster array for xswap devices

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] mm, swap: missing nr_clusters_mapped check in get_swap_device()
--

commit 59c7799e61f59ce4eb079135f6f31b57258c37bd
Author: Baoquan He <hebaoquan@kylinos.cn>

mm, swap: use a VM_SPARSE cluster array for xswap devices

This commit backs si->cluster_info for xswap devices with a VM_SPARSE
vmalloc area mapped initially to XSWAP_GROW_CLUSTERS clusters, and bounds
the cluster walks in find_next_to_unuse(), wait_for_allocation() and
free_swap_cluster_info() to si->nr_clusters_mapped.

> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index afd0e87185447..00c7b398bafec 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c

[ ... ]

> @@ -3902,6 +3969,69 @@ static int setup_swap_clusters_info(struct swap_info_struct *si,
>  	int err = -ENOMEM;
>  	unsigned long i;
>  
> +#ifdef CONFIG_XSWAP
> +	if (si->flags & SWP_XSWAP) {
> +		unsigned long size = PAGE_ALIGN(nr_clusters * sizeof(*cluster_info));
> +		struct vm_struct *vm;
> +
> +		vm = get_vm_area(size, VM_SPARSE);
> +		if (!vm)
> +			goto err;
> +
> +		cluster_info = vm->addr;
> +		si->cluster_vm = vm;
> +		si->nr_clusters_max = nr_clusters;
> +		si->cluster_info = cluster_info;
> +
> +		/* Must be initialized before xswap_map_clusters() locks it. */
> +		mutex_init(&si->xswap_lock);
> +
> +		if (xswap_map_clusters(si, 0, min_t(unsigned long,
> +					XSWAP_GROW_CLUSTERS, nr_clusters)))
> +			goto err_free_vm;

[Severity: High]
Can a lockless swap lookup fault on an unmapped page in si->cluster_info
when offset / SWAPFILE_CLUSTER >= si->nr_clusters_mapped?

setup_swap_clusters_info() maps only the first XSWAP_GROW_CLUSTERS
clusters while leaving si->max at the full device size, and
get_swap_device() only checks offset against si->max:

mm/swapfile.c:get_swap_device() {
    ...
	offset = swp_offset(entry);
	if (offset >= si->max)
		goto put_out;

	return si;
    ...
}

Callers such as do_swap_page() read a PTE locklessly before calling
get_swap_device(entry) and re-verify the PTE under the page table lock
only after looking up the swap cache.  If a stale PTE from a prior swapoff
and xswap activation (or after tail clusters are unmapped by commit
"mm, swap: add xswap shrink trigger and defer it to a workqueue") has
offset < si->max but offset / SWAPFILE_CLUSTER >= si->nr_clusters_mapped,
get_swap_device() succeeds and swap_cache_get_folio() ->
__swap_offset_to_cluster() dereferences an unmapped VM_SPARSE page in
si->cluster_info.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109055455.536925-1-baoquan.he@linux.dev?part=4