On Sat, 9 Dec 2023, Matthew Wilcox wrote:
> On Sat, Dec 09, 2023 at 01:35:45PM +0100, gregkh@xxxxxxxxxxxxxxxxxxx wrote:
> > The patch below does not apply to the 5.15-stable tree.
> > If someone wants it applied there, or to any other stable or longterm
> > tree, then please email the backport, including the original git commit
> > id to <stable@xxxxxxxxxxxxxxx>.
>
> This should do the job. It's not clear to me whether this bug remains
> latent on 5.15, so it may not be appropriate to apply. I defer to Hugh.
Yes, that's exactly it, thanks Matthew. I knew I was going to need to
look at this one, and even the 6.1 version, when they came out: because
my mods did change what a safe procedure is; but this and the 6.1
version are both good, because of the old pmd_devmap_trans_unstable()
check which follows in both of those trees.
As to whether it's needed in 5.15 and 6.1: I believe so. There's no
doubt that my changes made it very much easier to hit, but I think it
was a possibility before them. When I first wrote the commit message,
I was describing how: I think you need huge tmpfs, and MADV_DONTNEED
zapping the huge pmd under mmap_lock for read, racing with this fault
which, to come this way, would have needed to have been on a previous
page table which khugepaged has collapsed just before MADV_DONTNEED
and fault got mmap_lock for read.
Far fetched, and of course I could be wrong. But reading my original
commit message, I thought it was needlessly encouraging to bad actors,
so cut it out. What I'm most afraid of is the "(or some other symptom
in normal case of ptlock embedded not pointer)" - data corruption or
data leak perhaps.
I see Greg provides some new and helpfully explicit instructions below,
on how to manage sending replacement patches: I'll send a replacement
following those instructions (but might hit an issue at git send-email
stage - will send manually if so).
Hugh
>
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 81e28722edfa..84a5b0213e0e 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -3209,7 +3209,7 @@ static bool filemap_map_pmd(struct vm_fault *vmf, struct page *page)
> }
> }
>
> - if (pmd_none(*vmf->pmd)) {
> + if (pmd_none(*vmf->pmd) && vmf->prealloc_pte) {
> vmf->ptl = pmd_lock(mm, vmf->pmd);
> if (likely(pmd_none(*vmf->pmd))) {
> mm_inc_nr_ptes(mm);
>
> > To reproduce the conflict and resubmit, you may use the following commands:
> >
> > git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-5.15.y
> > git checkout FETCH_HEAD
> > git cherry-pick -x 9aa1345d66b8132745ffb99b348b1492088da9e2
> > # <resolve conflicts, build, test, etc.>
> > git commit -s
> > git send-email --to '<stable@xxxxxxxxxxxxxxx>' --in-reply-to '2023120945-citizen-library-9f46@gregkh' --subject-prefix 'PATCH 5.15.y' HEAD^..
> >
> > Possible dependencies:
> >
> > 9aa1345d66b8 ("mm: fix oops when filemap_map_pmd() without prealloc_pte")
> > 03c4f20454e0 ("mm: introduce pmd_install() helper")
> >
> > thanks,
> >
> > greg k-h
> >
> > ------------------ original commit in Linus's tree ------------------
> >
> > >From 9aa1345d66b8132745ffb99b348b1492088da9e2 Mon Sep 17 00:00:00 2001
> > From: Hugh Dickins <hughd@xxxxxxxxxx>
> > Date: Fri, 17 Nov 2023 00:49:18 -0800
> > Subject: [PATCH] mm: fix oops when filemap_map_pmd() without prealloc_pte
> > MIME-Version: 1.0
> > Content-Type: text/plain; charset=UTF-8
> > Content-Transfer-Encoding: 8bit
> >
> > syzbot reports oops in lockdep's __lock_acquire(), called from
> > __pte_offset_map_lock() called from filemap_map_pages(); or when I run the
> > repro, the oops comes in pmd_install(), called from filemap_map_pmd()
> > called from filemap_map_pages(), just before the __pte_offset_map_lock().
> >
> > The problem is that filemap_map_pmd() has been assuming that when it finds
> > pmd_none(), a page table has already been prepared in prealloc_pte; and
> > indeed do_fault_around() has been careful to preallocate one there, when
> > it finds pmd_none(): but what if *pmd became none in between?
> >
> > My 6.6 mods in mm/khugepaged.c, avoiding mmap_lock for write, have made it
> > easy for *pmd to be cleared while servicing a page fault; but even before
> > those, a huge *pmd might be zapped while a fault is serviced.
> >
> > The difference in symptomatic stack traces comes from the "memory model"
> > in use: pmd_install() uses pmd_populate() uses page_to_pfn(): in some
> > models that is strict, and will oops on the NULL prealloc_pte; in other
> > models, it will construct a bogus value to be populated into *pmd, then
> > __pte_offset_map_lock() oops when trying to access split ptlock pointer
> > (or some other symptom in normal case of ptlock embedded not pointer).
> >
> > Link: https://lore.kernel.org/linux-mm/20231115065506.19780-1-jose.pekkarinen@xxxxxxxxxxx/
> > Link: https://lkml.kernel.org/r/6ed0c50c-78ef-0719-b3c5-60c0c010431c@xxxxxxxxxx
> > Fixes: f9ce0be71d1f ("mm: Cleanup faultaround and finish_fault() codepaths")
> > Signed-off-by: Hugh Dickins <hughd@xxxxxxxxxx>
> > Reported-and-tested-by: syzbot+89edd67979b52675ddec@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Closes: https://lore.kernel.org/linux-mm/0000000000005e44550608a0806c@xxxxxxxxxx/
> > Reviewed-by: David Hildenbrand <david@xxxxxxxxxx>
> > Cc: Jann Horn <jannh@xxxxxxxxxx>,
> > Cc: José Pekkarinen <jose.pekkarinen@xxxxxxxxxxx>
> > Cc: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
> > Cc: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
> > Cc: <stable@xxxxxxxxxxxxxxx> [5.12+]
> > Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> >
> > diff --git a/mm/filemap.c b/mm/filemap.c
> > index 32eedf3afd45..f1c8c278310f 100644
> > --- a/mm/filemap.c
> > +++ b/mm/filemap.c
> > @@ -3371,7 +3371,7 @@ static bool filemap_map_pmd(struct vm_fault *vmf, struct folio *folio,
> > }
> > }
> >
> > - if (pmd_none(*vmf->pmd))
> > + if (pmd_none(*vmf->pmd) && vmf->prealloc_pte)
> > pmd_install(mm, vmf->pmd, &vmf->prealloc_pte);
> >
> > return false;
> >
