Re: [PATCH] mm/highmem: refuse to copy hwpoisoned folios in the !copy_mc fallbacks

[Huang, Ying]

"Garg, Shivank" <shivankg@xxxxxxx> writes:

> On Mon, 2026-10-26 at 19:21 +0800, Huang, Ying wrote:
>> Yuanhe Shu <xiangzao@xxxxxxxxxxxxxxxxx> writes:
>> 
>> > Where copy_mc_to_kernel is defined, copy_mc_user_highpage() and
>> > copy_mc_highpage() stop at a hardware error during the copy, report
>> > the not-copied bytes and queue the source page for memory_failure().
>> > The !copy_mc fallbacks return 0 after a plain copy instead.  With
>> > PG_hwpoison set while the data stays readable - software-injected
>> > poison (MADV_HWPOISON, the debugfs corrupt-pfn injector) or a
>> > transient hardware error - there is no fault to recover from: the
>> > copy completes, the destination folio carries no poison marker, and
>> > the poison is consumed silently, the same poison consumption
>> > commit 841a8bfcbad9 ("mm: prevent poison consumption when splitting
>> > THP") refused to risk on the split path.
>> 
>> As far as I know, software-injected poison is used for debugging only ,
>> so I don't think it's that important.  Hardware poison state in DDR is
>> sticky; that is, even a transient hardware error will cause a permanent
>> hardware poison state until a reset.
>> 
>> Therefore, the primary value of this patch should be to reduce machine
>> checks (and panics) during memory copies.  If so, we should place the
>> check code before copying.
>> 
>> > Check the source folio with folio_contain_hwpoisoned_page() in both
>> > fallbacks and return PAGE_SIZE, the not-copied-bytes convention of
>> > the real copy_mc paths, so the callers take their existing error
>> > paths: the COW and fork paths fail with -EHWPOISON, KSM and
>> > khugepaged abort, and folio_mc_copy() propagates to the migration
>> > core, where this composes with the separate migration-side checks.
>> > On archs with copy_mc this code does not compile.
>> > 
>> > The check runs after the copy: a check before it would leave open
>> > the window between itself and the copy.  A flag set before or during
>> > the copy is caught, whatever state the data is in.  A late software
>> > injection cannot have corrupted the completed copy, and a synchronous
>> > hardware error takes an arch without copy_mc down at the access; the
>> > residue no flag check, before or after, can catch is corruption whose
>> > hardware report lands after the last check.  Closing that would need
>> > architecture support, as discussed.
>> > 
>> > The fallbacks do not queue memory_failure() on refusal: the flag is
>> > already set, and memory failure either knows about the page or has
>> > abandoned it.
>> > 
>> > Suggested-by: "Huang, Ying" <ying.huang@xxxxxxxxxxxxxxxxx>
>> > Suggested-by: Kaitao Cheng <kaitao.cheng@xxxxxxxxx>
>> > Assisted-by: LLM
>> > Signed-off-by: Yuanhe Shu <xiangzao@xxxxxxxxxxxxxxxxx>
>> > Link: https://lore.kernel.org/r/877bjowf9i.fsf@DESKTOP-5N7EMDA/
>> > ---
>> >  include/linux/highmem.h | 9 +++++++++
>> >  1 file changed, 9 insertions(+)
>> > 
>> > diff --git a/include/linux/highmem.h b/include/linux/highmem.h
>> > index d7aac9de1c8a..f90f0c6204a4 100644
>> > --- a/include/linux/highmem.h
>> > +++ b/include/linux/highmem.h
>> > @@ -474,12 +474,21 @@ static inline int copy_mc_user_highpage(struct page *to, struct page *from,
>> >  					unsigned long vaddr, struct vm_area_struct *vma)
>> >  {
>> >  	copy_user_highpage(to, from, vaddr, vma);
>> > +	/*
>> > +	 * The flag can be set while the copy runs, so check after it;
>> > +	 * a flag landing later can no longer have corrupted this copy.
>> > +	 */
>> > +	if (folio_contain_hwpoisoned_page(page_folio(from)))
>> > +		return PAGE_SIZE;
>> 
>> Why not just use PageHWPoison()?
>
> This will miss out on hugetlb folios I think. is_page_hwpoison() is more
> appropriate as it covers hugetlb cases as well.

Yes, that is better.  Thanks for pointing that out.

---
Best Regards,
Huang, Ying