linux-kernel - Re: [PATCH v3] mm/page_isolation: fix potential missing call to unset_migratetype

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: <454bd51f-d7ee-6304-af23-7c95874f8890@redhat.com>
Date:   Tue, 14 Sep 2021 20:13:28 +0200
From:   David Hildenbrand <david@...hat.com>
To:     Miaohe Lin <linmiaohe@...wei.com>, akpm@...ux-foundation.org
Cc:     mhocko@...e.com, vbabka@...e.cz, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] mm/page_isolation: fix potential missing call to
 unset_migratetype_isolate()

On 14.09.21 13:43, Miaohe Lin wrote:
> In start_isolate_page_range() undo path, pfn_to_online_page() just checks
> the first pfn in a pageblock while __first_valid_page() will traverse the
> pageblock until the first online pfn is found. So we may miss the call to
> unset_migratetype_isolate() in undo path and pages will remain isolated
> unexpectedly. Fix this by calling undo_isolate_page_range() and this will
> also help to simplify the code further. Note we shouldn't ever trigger it
> because MAX_ORDER-1 aligned pfn ranges shouldn't contain memory holes now.
> 
> Fixes: 2ce13640b3f4 ("mm: __first_valid_page skip over offline pages")
> Signed-off-by: Miaohe Lin <linmiaohe@...wei.com>

I read Michals reply, however, I am quite conservative with Fixes: tags. 
If there is nothing to fix, there is no BUG and the patch consequently 
merely a cleanup.

I'd have gone with a patch description/subject as follows:

"
mm/page_isolation: cleanup start_isolate_page_range()

We can heavily simplify the code by reusing undo_isolate_page_range().

Note that this also tackles a theoretical issue that would have been a 
real BUG before commit c5e79ef561b0 ("mm/memory_hotplug.c: don't allow 
to online/offline memory blocks with holes"). In 
start_isolate_page_range() undo path, pfn_to_online_page() just checks
the first pfn in a pageblock while __first_valid_page() will traverse 
the pageblock until the first online pfn is found. So we may miss the 
call to unset_migratetype_isolate() in undo path and pages will remain 
isolated unexpectedly.

Nowadays, start_isolate_page_range() never gets called on ranges that 
might contain memory holes. Consequently, this patch is not a fix but a 
cleanup.
"

Anyhow, whatever the other people prefer, no strong opinion.

Reviewed-by: David Hildenbrand <david@...hat.com>

-- 
Thanks,

David / dhildenb