Re: [PATCH v3] mm: introduce copy_mc_highpages() and batch copy in folio_mc_copy()

[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:
- [Critical] mm: missing hwpoison check in copy_mc_user_highpage() and
  copy_mc_highpage() [1]
--

commit fae95970ccf2607d670f06862e40d3ba9313200a
Author: Shivank Garg <shivankg@amd.com>

mm: introduce copy_mc_highpages() and batch copy in folio_mc_copy()

This commit introduces copy_mc_highpages() to copy page ranges with a
single copy_mc_to_kernel() call when possible and updates folio_mc_copy()
to use it.

> diff --git a/include/linux/highmem.h b/include/linux/highmem.h
> index d7aac9de1c8a4..a79e9d68359c8 100644
> --- a/include/linux/highmem.h
> +++ b/include/linux/highmem.h
> @@ -484,6 +484,57 @@ static inline int copy_mc_highpage(struct page *to, struct page *from)
>  }
>  #endif
>  
> +/**
> + * copy_mc_highpages() - copy a page range with machine check handling
> + * @to: first destination page
> + * @from: first source page
> + * @nr_pages: number of pages
> + *
> + * Range variant of copy_mc_highpage(). On a machine check, the poisoned
> + * source page is queued for memory_failure().
> + *
> + * Context: Process context. May sleep.
> + * Return: 0 on success, -EHWPOISON if a machine check was handled.
> + */
> +static inline int copy_mc_highpages(struct page *to, struct page *from,
> +		unsigned long nr_pages)
> +{
> +	unsigned long len = nr_pages << PAGE_SHIFT;
> +	unsigned long i, ret;
> +
> +	might_sleep();
> +
> +	/*
> +	 * When copying we want to operate on the largest extent possible to
> +	 * allow for architecture specific extent based optimizations.
> +	 *
> +	 * copy_mc_to_kernel() does not call cond_resched(). That is fine on
> +	 * preemptible kernels, where the scheduler can preempt the copy at
> +	 * any point, so copy the whole range at once if copy_mc_to_kernel() is
> +	 * available and the pages are in the linear map. Otherwise, copy one
> +	 * page at a time and reschedule in between.
> +	 */
> +	if (IS_ENABLED(CONFIG_ARCH_HAS_COPY_MC) && IS_ENABLED(CONFIG_PREEMPTION) &&
> +	    !IS_ENABLED(CONFIG_HIGHMEM)) {
> +		ret = copy_mc_to_kernel(page_address(to), page_address(from), len);
> +		if (ret) {
> +			memory_failure_queue(page_to_pfn(from) +
> +					     ((len - ret) >> PAGE_SHIFT), 0);
> +			return -EHWPOISON;
> +		}
> +		for (i = 0; i < nr_pages; i++)
> +			kmsan_copy_page_meta(to + i, from + i);
> +		return 0;
> +	}
> +
> +	for (i = 0; i < nr_pages; i++) {
> +		cond_resched();
> +		if (copy_mc_highpage(to + i, from + i))
> +			return -EHWPOISON;
> +	}

[Severity: Critical]
This problem wasn't introduced by this patch, but can copy_mc_highpage()
and copy_mc_user_highpage() (as well as the new copy_mc_highpages() fast
path) return 0 when the source page or its containing folio is already
marked PG_hwpoison [1]?

Neither helper checks PageHWPoison() or folio_contain_hwpoisoned_page()
before copying.  copy_mc_to_kernel() only catches synchronous #MC
exceptions raised during the copy itself (and falls back to plain memcpy()
when unavailable or on x86 without X86_FEATURE_ERMS or
copy_mc_fragile_enabled).

If a source folio was poisoned via software injection (such as
MADV_HWPOISON) or a prior memory_failure() where reading does not trigger
a synchronous #MC, callers such as folio_mc_copy() or wp_page_copy() treat
the copy as successful and copy corrupted data into a fresh destination
folio without PG_hwpoison set, or trigger an unrecoverable kernel #MC when
copy_mc_to_kernel() is not active.

[1] https://sashiko.dev/#/bug/linux-3af4aa7d-d182-430b-b982-583f6aea4594

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-batpagecopy-v3-1-297f5b2fd28e@amd.com?part=1