linux-kernel - Re: [PATCH v2 4/5] mm/hwpoison: Fix incorrect "not recovered" report for recovered clean pages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ddd769c2-a17d-9e34-822d-66f72bd654ac@huawei.com>
Date: Wed, 19 Feb 2025 14:34:50 +0800
From: Miaohe Lin <linmiaohe@...wei.com>
To: Shuai Xue <xueshuai@...ux.alibaba.com>
CC: <tglx@...utronix.de>, <mingo@...hat.com>, <dave.hansen@...ux.intel.com>,
	<x86@...nel.org>, <hpa@...or.com>, <akpm@...ux-foundation.org>,
	<peterz@...radead.org>, <jpoimboe@...nel.org>, <linux-edac@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <linux-mm@...ck.org>,
	<baolin.wang@...ux.alibaba.com>, <tianruidong@...ux.alibaba.com>,
	<tony.luck@...el.com>, <bp@...en8.de>, <nao.horiguchi@...il.com>
Subject: Re: [PATCH v2 4/5] mm/hwpoison: Fix incorrect "not recovered" report
 for recovered clean pages

On 2025/2/17 14:33, Shuai Xue wrote:
> When an uncorrected memory error is consumed there is a race between
> the CMCI from the memory controller reporting an uncorrected error
> with a UCNA signature, and the core reporting and SRAR signature
> machine check when the data is about to be consumed.
> 
> If the CMCI wins that race, the page is marked poisoned when
> uc_decode_notifier() calls memory_failure(). For dirty pages,
> memory_failure() invokes try_to_unmap() with the TTU_HWPOISON flag,
> converting the PTE to a hwpoison entry. As a result,
> kill_accessing_process():
> 
> - call walk_page_range() and return 1 regardless of whether
>   try_to_unmap() succeeds or fails,
> - call kill_proc() to make sure a SIGBUS is sent
> - return -EHWPOISON to indicate that SIGBUS is already sent to the
>   process and kill_me_maybe() doesn't have to send it again.
> 
> However, for clean pages, the TTU_HWPOISON flag is cleared, leaving the
> PTE unchanged and not converted to a hwpoison entry. Conversely, for
> clean pages where PTE entries are not marked as hwpoison,
> kill_accessing_process() returns -EFAULT, causing kill_me_maybe() to
> send a SIGBUS.
> 
> Console log looks like this:
> 
>     Memory failure: 0x827ca68: corrupted page was clean: dropped without side effects
>     Memory failure: 0x827ca68: recovery action for clean LRU page: Recovered
>     Memory failure: 0x827ca68: already hardware poisoned
>     mce: Memory error not recovered
> 
> To fix it, return 0 for "corrupted page was clean", preventing an
> unnecessary SIGBUS.
> 
> Fixes: 046545a661af ("mm/hwpoison: fix error page recovered but reported "not recovered"")
> Signed-off-by: Shuai Xue <xueshuai@...ux.alibaba.com>
> Cc: stable@...r.kernel.org
> ---
>  mm/memory-failure.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 995a15eb67e2..b037952565be 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -881,12 +881,17 @@ static int kill_accessing_process(struct task_struct *p, unsigned long pfn,
>  	mmap_read_lock(p->mm);
>  	ret = walk_page_range(p->mm, 0, TASK_SIZE, &hwpoison_walk_ops,
>  			      (void *)&priv);
> +	/*
> +	 * ret = 1 when CMCI wins, regardless of whether try_to_unmap()
> +	 * succeeds or fails, then kill the process with SIGBUS.
> +	 * ret = 0 when poison page is a clean page and it's dropped, no
> +	 * SIGBUS is needed.
> +	 */
>  	if (ret == 1 && priv.tk.addr)
>  		kill_proc(&priv.tk, pfn, flags);
> -	else
> -		ret = 0;
>  	mmap_read_unlock(p->mm);
> -	return ret > 0 ? -EHWPOISON : -EFAULT;
> +
> +	return ret > 0 ? -EHWPOISON : 0;

The caller kill_me_maybe will do set_mce_nospec + sync_core again.

static void kill_me_maybe(struct callback_head *cb)
{
	struct task_struct *p = container_of(cb, struct task_struct, mce_kill_me);
	int flags = MF_ACTION_REQUIRED;
	...
	ret = memory_failure(pfn, flags);
	if (!ret) {
		set_mce_nospec(pfn);
		sync_core();
		return;
	}

Is this expected?

Thanks.
.