linux-kernel - Re: [PATCH] hv_balloon: avoid touching uninitialized struct page during tail onlining

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]

Date:   Tue, 08 Jan 2019 10:42:56 +0100
From:   Vitaly Kuznetsov <vkuznets@...hat.com>
To:     David Hildenbrand <david@...hat.com>, devel@...uxdriverproject.org
Cc:     Sasha Levin <sashal@...nel.org>,
        "K. Y. Srinivasan" <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        linux-kernel@...r.kernel.org, Dexuan Cui <decui@...rosoft.com>
Subject: Re: [PATCH] hv_balloon: avoid touching uninitialized struct page during tail onlining

David Hildenbrand <david@...hat.com> writes:

> On 07.01.19 14:44, Vitaly Kuznetsov wrote:
>> David Hildenbrand <david@...hat.com> writes:
>> 
...
>>> On 04.01.19 15:19, Vitaly Kuznetsov wrote:
>>>>  			if (start_pfn > has->start_pfn &&
>>>> -			    !PageReserved(pfn_to_page(start_pfn - 1)))
>>>> +			    online_section_nr(pfn_to_section_nr(start_pfn)))
>>>>  				hv_bring_pgs_online(has, start_pfn, pgs_ol);
>>>>  
>>>>  		}
>>>>
>>>
>>> I wonder if you should use pfn_to_online_page() and check for PageOffline().
>>>
>>> (I guess online_section_nr() should also do the trick)
>> 
>> I'm worried a bit about racing with mm code here as we're not doing
>> mem_hotplug_begin()/done() so I'd slightly prefer keeping
>> online_section_nr() (pfn_to_online_page() also uses it but then it gets
>> to the particular struct page). Moreover, with pfn_to_online_page() we
>> will be looking at some other pfn - because the start_pfn is definitelly
>> offline (pre-patch we were looking at start_pfn-1). Just looking at the
>> whole section seems cleaner.
>
> Fine with me. I guess the section can never be offlined as it still
> contains reserved pages if not fully "fake-onlined" by HV code already.
>
> But we could still consider mem_hotplug_begin()/done() as we could have
> a online section although online_pages() has not completed yet. So we
> could actually touch some "semi onlined section".

Yes, exactly, if we race with section onlining here we may never online
the tail so it will remain 'semi onlined'. I'm going to propose
exporting mem_hotplug_begin()/done() to modules to fix this (in a
separate patch because I anticipate some pushback :-)

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

Thanks!

-- 
Vitaly