Discussion on race between freed page_ext access and memory offline operation

[Charan Teja Kalla <quic_charante@quicinc.com>]

The below race between page_ext and online/offline of the respective
memory blocks will cause use-after-free on the access of page_ext structure.

process1			    process2
---------			    ---------
a)doing /proc/page_owner            doing memory offline	
			            through offline_pages

b)PageBuddy check is failed
thus proceed to get the
page_owner information
through page_ext access.
page_ext = lookup_page_ext(page);
				
				  migrate_pages();
				 ................
				Since all pages are successfully
				migrated as part of the offline
				operation,send MEM_OFFLINE notification
				where for page_ext it calls:
				offline_page_ext()-->
				  __free_page_ext()-->
				    free_page_ext()-->
				      vfree(ms->page_ext)
				mem_section->page_ext = NULL

c) Check for the PAGE_EXT flags
in the page_ext->flags access
results into the use-after-free(leading
to the translation faults).

As mentioned above, there is really no synchronization between page_ext
access and its freeing in the memory_offline.  The above is just one
example but the problem persists in the other paths too involving
page_ext->flags access(eg: page_is_idle()).

The memory offline steps(roughly) on a memory block is as below:
1) Isolate all the pages
2) while(1)
  try free the pages to buddy.(->free_list[MIGRATE_ISOLATE])
3) delete the pages from this buddy list.
4) Then free page_ext.(Note: The struct page is still alive as it is
freed only during hot remove of the memory which frees the memmap, which
steps the user might not perform).

This design leads to the state where struct page is alive but the struct
page_ext is freed, where the later is ideally part of the former which
just representing the page_flags. This seems to be a wrong design where
'struct page' as a whole is not accessible(Thanks to Minchan for
pointing this out).

Some solutions we think are:
----------------------------
1) Take the mem_hotplug_lock read_lock every time page_ext access.

2) Take the extra refcount on the page every time page_ext access is
made, so that parallel offline operation can't free the page to buddy.

3) Change the design where the page_ext is valid as long as the struct
page is alive.

Any other inputs here?

PS: This bug is uncovered while fixing the same page_ext access issue
with the page
pinner(https://lore.kernel.org/linux-mm/20211206184730.858850-1-minchan@kernel.org/)
on Andorid.


Thanks,
Charan