[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAOUHufb4O7oCsGcH5VcSoAw5cUiwYjGCfvLBHPZgo-G=HtiLVw@mail.gmail.com>
Date: Thu, 27 Jun 2024 17:04:58 -0600
From: Yu Zhao <yuzhao@...gle.com>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: Muchun Song <muchun.song@...ux.dev>, David Hildenbrand <david@...hat.com>,
Frank van der Linden <fvdl@...gle.com>, "Matthew Wilcox (Oracle)" <willy@...radead.org>, Peter Xu <peterx@...hat.com>,
Yang Shi <yang@...amperecomputing.com>, linux-mm@...ck.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH mm-unstable v2] mm/hugetlb_vmemmap: fix race with
speculative PFN walkers
On Thu, Jun 27, 2024 at 4:47 PM Andrew Morton <akpm@...ux-foundation.org> wrote:
>
> On Thu, 27 Jun 2024 16:27:05 -0600 Yu Zhao <yuzhao@...gle.com> wrote:
>
> > While investigating HVO for THPs [1], it turns out that speculative
> > PFN walkers like compaction can race with vmemmap modifications, e.g.,
> >
> > CPU 1 (vmemmap modifier) CPU 2 (speculative PFN walker)
> > ------------------------------- ------------------------------
> > Allocates an LRU folio page1
> > Sees page1
> > Frees page1
> >
> > Allocates a hugeTLB folio page2
> > (page1 being a tail of page2)
> >
> > Updates vmemmap mapping page1
> > get_page_unless_zero(page1)
> >
> > Even though page1->_refcount is zero after HVO, get_page_unless_zero()
> > can still try to modify this read-only field, resulting in a crash.
>
> Ah. So we should backport this into earlier kernels, yes?
>
> Are we able to identify a Fixes: for this? Looks difficult.
>
> This seems quite hard to trigger. Do any particular userspace actions
> invoke the race?
Yes, *very* hard to trigger:
1. Most hugeTLB use cases I know of are static, i.e., reserved at boot
time, because allocating at runtime is not reliable at all.
2. On top of that, someone has to be very unlucky to get tripped over
above, because the race window is so small -- I wasn't able to trigger
it with a stress testing that does nothing but that (with THPs
though).
So I don't think it's worth cc'ing stable, unless Muchun recommends.
Powered by blists - more mailing lists