On 3/31/22 12:26, Muchun Song wrote: > 1st concern: > ''' > But what happens when a hot remove section's vmemmap area (which is > being teared down) is nearby another vmemmap area which is either created > or being destroyed for HugeTLB alloc/free purpose. As you mentioned > HugeTLB pages inside the hot remove section might be safe. But what about > other HugeTLB areas whose vmemmap area shares page table entries with > vmemmap entries for a section being hot removed ? Massive HugeTLB alloc > /use/free test cycle using memory just adjacent to a memory hotplug area, > which is always added and removed periodically, should be able to expose > this problem. > ''' > > Answer: At the time memory is removed, all HugeTLB pages either have been > migrated away or dissolved. So there is no race between memory hot remove > and free_huge_page_vmemmap(). Therefore, HugeTLB pages inside the hot > remove section is safe. Let's talk your question "what about other HugeTLB pages inside the memory range is safe but concern is about the vmemmap mapping for the HugeTLB which might share intermediate entries with vmemmap mapping for the memory range/section being removed. > HugeTLB areas whose vmemmap area shares page table entries with vmemmap > entries for a section being hot removed ?", the question is not Right. > established. The minimal granularity size of hotplug memory 128MB (on > arm64, 4k base page), any HugeTLB smaller than 128MB is within a section, > then, there is no share PTE page tables between HugeTLB in this section 128MB is the hot removable granularity but, its corresponding vmemmap range is smaller i.e (128MB/4K) * sizeof(struct page). Memory section getting hot removed (its vmemmap mapping being teared down) along with HugeTLB (on another section) vmemmap remap operation, could not collide while inside vmemmap mapping areas on init_mm ? > and ones in other sections and a HugeTLB page could not cross two > sections. In this case, the section cannot be freed. Any HugeTLB bigger Right, they dont cross into two different sections. > than 128MB (section size) whose vmemmap pages is an integer multiple of > 2MB (PMD-mapped). As long as: > > 1) HugeTLBs are naturally aligned, power-of-two sizes > 2) The HugeTLB size >= the section size > 3) The HugeTLB size >= the vmemmap leaf mapping size > > Then a HugeTLB will not share any leaf page table entries with *anything > else*, but will share intermediate entries. In this case, at the time memory > is removed, all HugeTLB pages either have been migrated away or dissolved. > So there is also no race between memory hot remove and > free_huge_page_vmemmap(). If they just share intermediate entries, free_empty_tables() will not free up page table pages, as there will be valid non-zero entries in them. But the problem here is not UAF, its accessing wrong entries and crashing while de-referncing the pointer. Hence I am wondering if no such scenario can be possible. > > 2nd concern: > ''' > differently, not sure if ptdump would require any synchronization. > > Dumping an wrong value is probably okay but crashing because a page table > entry is being freed after ptdump acquired the pointer is bad. On arm64, > ptdump() is protected against hotremove via [get|put]_online_mems(). > ''' > > Answer: The ptdump should be fine since vmemmap_remap_free() only exchanges > PTEs or splits the PMD entry (which means allocating a PTE page table). Both > operations do not free any page tables (PTE), so ptdump cannot run into a > UAF on any page tables. The worst case is just dumping an wrong value. Okay, fair enough. ptdump() might be just okay.
