On Thu, Jan 7, 2021 at 4:41 PM Michal Hocko [off-list ref] wrote:

On Thu 07-01-21 13:39:38, Muchun Song wrote:

quoted

On Thu, Jan 7, 2021 at 12:56 AM Michal Hocko [off-list ref] wrote:

quoted

On Wed 06-01-21 16:47:36, Muchun Song wrote:

quoted

There is a race condition between __free_huge_page()
and dissolve_free_huge_page().

CPU0:                         CPU1:

// page_count(page) == 1
put_page(page)
  __free_huge_page(page)
                              dissolve_free_huge_page(page)
                                spin_lock(&hugetlb_lock)
                                // PageHuge(page) && !page_count(page)
                                update_and_free_page(page)
                                // page is freed to the buddy
                                spin_unlock(&hugetlb_lock)
    spin_lock(&hugetlb_lock)
    clear_page_huge_active(page)
    enqueue_huge_page(page)
    // It is wrong, the page is already freed
    spin_unlock(&hugetlb_lock)

The race windows is between put_page() and spin_lock() which
is in the __free_huge_page().

The race window reall is between put_page and dissolve_free_huge_page.
And the result is that the put_page path would clobber an unrelated page
(either free or already reused page) which is quite serious.
Fortunatelly pages are dissolved very rarely. I believe that user would
require to be privileged to hit this by intention.

quoted

We should make sure that the page is already on the free list
when it is dissolved.

Another option would be to check for PageHuge in __free_huge_page. Have
you considered that rather than add yet another state? The scope of the
spinlock would have to be extended. If that sounds more tricky then can
we check the page->lru in the dissolve path? If the page is still
PageHuge and reference count 0 then there shouldn't be many options
where it can be queued, right?

Did you mean that we iterate over the free list to check whether
the page is on the free list?

No I meant to check that the page is enqueued which along with ref count
= 0 should mean it has been released to the pool unless I am missing
something.

The page can be on the free list or active list or empty when it
is freed to the pool. How to check whether it is on the free list?

--
Michal Hocko
SUSE Labs