linux-kernel - Re: [PATCH 4/4] mm: memory-failure: add PageOffline() check

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: <20230720010953.GC1583723@ik1-406-35019.vs.sakura.ne.jp>
Date:   Thu, 20 Jul 2023 10:09:53 +0900
From:   Naoya Horiguchi <naoya.horiguchi@...ux.dev>
To:     Miaohe Lin <linmiaohe@...wei.com>
Cc:     akpm@...ux-foundation.org, naoya.horiguchi@....com,
        shy828301@...il.com, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/4] mm: memory-failure: add PageOffline() check

On Sat, Jul 15, 2023 at 11:17:29AM +0800, Miaohe Lin wrote:
> Memory failure is not interested in logically offlined page. Skip this
> type of pages.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@...wei.com>
> ---
>  mm/memory-failure.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 42e63b0ab5f7..ed79b69837de 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -1559,7 +1559,7 @@ static bool hwpoison_user_mappings(struct page *p, unsigned long pfn,
>  	 * Here we are interested only in user-mapped pages, so skip any
>  	 * other types of pages.
>  	 */
> -	if (PageReserved(p) || PageSlab(p) || PageTable(p))
> +	if (PageReserved(p) || PageSlab(p) || PageTable(p) || PageOffline(p))

hwpoison_user_mappings() is called after some checks are done, so I'm not
sure that it's the right place to check PageOffline().
We could check it before setting PageHWPoison() as we do at the beginning of
memory_failure() around pfn_to_online_page().  Does it make sense?

Thanks,
Naoya Horiguchi

>  		return true;
>  	if (!(PageLRU(hpage) || PageHuge(p)))
>  		return true;
> @@ -2513,7 +2513,8 @@ int unpoison_memory(unsigned long pfn)
>  		goto unlock_mutex;
>  	}
>  
> -	if (folio_test_slab(folio) || PageTable(&folio->page) || folio_test_reserved(folio))
> +	if (folio_test_slab(folio) || PageTable(&folio->page) ||
> +	    folio_test_reserved(folio) || PageOffline(&folio->page))
>  		goto unlock_mutex;
>  
>  	/*
> -- 
> 2.33.0
> 
> 
>