linux-kernel - Re: [PATCH v2 5/5] hugetlb: Allow faults to be handled under the VMA lock

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] [day] [month] [year] [list]

Message-ID: <CAOzc2pyt=3UwRt6a8L-wJGAcQFJr_ZNeFEwiGapx9KktuYC+BQ@mail.gmail.com>
Date: Thu, 22 Feb 2024 09:27:00 -0800
From: Vishal Moola <vishal.moola@...il.com>
To: Matthew Wilcox <willy@...radead.org>
Cc: linux-mm@...ck.org, linux-kernel@...r.kernel.org, 
	akpm@...ux-foundation.org, muchun.song@...ux.dev, 
	Cyril Hrubis <chrubis@...e.cz>, Jan Stancek <jstancek@...hat.com>, Petr Vorel <pvorel@...e.cz>, 
	ltp@...ts.linux.it
Subject: Re: [PATCH v2 5/5] hugetlb: Allow faults to be handled under the VMA lock

On Wed, Feb 21, 2024 at 7:55 PM Matthew Wilcox <willy@...radead.org> wrote:
>
> On Wed, Feb 21, 2024 at 03:47:32PM -0800, Vishal Moola (Oracle) wrote:
> > Hugetlb can now safely handle faults under the VMA lock, so allow it to
> > do so.
> >
> > This patch may cause ltp hugemmap10 to "fail". Hugemmap10 tests hugetlb
> > counters, and expects the counters to remain unchanged on failure to
> > handle a fault.
> >
> > In hugetlb_no_page(), vmf_anon_prepare() may bailout with no anon_vma
> > under the VMA lock after allocating a folio for the hugepage. In
> > free_huge_folio(), this folio is completely freed on bailout iff there
> > is a surplus of hugetlb pages. This will remove a folio off the freelist
> > and decrement the number of hugepages while ltp expects these counters
> > to remain unchanged on failure.
> >
> > Originally this could only happen due to OOM failures, but now it may
> > also occur after we allocate a hugetlb folio without a suitable anon_vma
> > under the VMA lock. This should only happen for the first freshly
> > allocated hugepage in this vma.
>
> Hmm, so it's a bug in LTP.  Have you talked to the LTP people about it
> (cc's added)?
>
> Also, did you try moving the anon_vma check befor the folio allocation
> so that you can bail out without disturbing the counters?

Moving the check before folio allocation occurs keeps the folios on the
freelist so the counters remain static as expected.

If we are looking at a shareable vma, hugetlb_no_page() does not need
this check at all, so I left the check where it is. We could definitely place
the anon_vma check above allocation, it would just make the code
slightly more complicated - needing another variable (or reassigning
ret in various places) as well as adding another potentially unnecessary
vma flag check.

> > Signed-off-by: Vishal Moola (Oracle) <vishal.moola@...il.com>
>
> Reviewed-by: Matthew Wilcox (Oracle) <willy@...radead.org>