linux-kernel - Re: [PATCH] mm: memory-failure: fix unexpected return value in soft_offline

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: <20230628110618.GA1248243@ik1-406-35019.vs.sakura.ne.jp>
Date:   Wed, 28 Jun 2023 20:06:18 +0900
From:   Naoya Horiguchi <naoya.horiguchi@...ux.dev>
To:     Miaohe Lin <linmiaohe@...wei.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>, naoya.horiguchi@....com,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm: memory-failure: fix unexpected return value in
 soft_offline_page()

On Wed, Jun 28, 2023 at 09:56:38AM +0800, Miaohe Lin wrote:
> On 2023/6/28 3:30, Andrew Morton wrote:
> > On Tue, 27 Jun 2023 19:28:08 +0800 Miaohe Lin <linmiaohe@...wei.com> wrote:
> > 
> >> When page_handle_poison() fails to handle the hugepage or free page in
> >> retry path, soft_offline_page() will return 0 while -EBUSY is expected
> >> in this case.
> > 
> > What are the user visible effects of the bug?
> 
> The user will think soft_offline_page succeeds while it failed in fact. So user
> will not try again later in this case.

I think that it's helpful to put this in patch descrition so that maintainers can
easily guess the impact of this patch.

Anyway, the patch looks good to me, thank you.

Acked-by: Naoya Horiguchi <naoya.horiguchi@....com>

> 
> > 
> >> Fixes: b94e02822deb ("mm,hwpoison: try to narrow window race for free pages")
> >>
> >> ...
> >>
> >> --- a/mm/memory-failure.c
> >> +++ b/mm/memory-failure.c
> >> @@ -2737,10 +2737,13 @@ int soft_offline_page(unsigned long pfn, int flags)
> >>  	if (ret > 0) {
> >>  		ret = soft_offline_in_use_page(page);
> >>  	} else if (ret == 0) {
> >> -		if (!page_handle_poison(page, true, false) && try_again) {
> >> -			try_again = false;
> >> -			flags &= ~MF_COUNT_INCREASED;
> >> -			goto retry;
> >> +		if (!page_handle_poison(page, true, false)) {
> >> +			if (try_again) {
> >> +				try_again = false;
> >> +				flags &= ~MF_COUNT_INCREASED;
> >> +				goto retry;
> >> +			}
> >> +			ret = -EBUSY;
> >>  		}
> >>  	}
> > .
> > 
> 
> 
>