From: Peter Xu <peterx@redhat.com>
To: Mike Kravetz <mike.kravetz@oracle.com>
Cc: Hugh Dickins <hughd@google.com>,
Axel Rasmussen <axelrasmussen@google.com>,
Yang Shi <shy828301@gmail.com>,
Matthew Wilcox <willy@infradead.org>,
syzbot <syzbot+152d76c44ba142f8992b@syzkaller.appspotmail.com>,
akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, llvm@lists.linux.dev, nathan@kernel.org,
ndesaulniers@google.com, songmuchun@bytedance.com,
syzkaller-bugs@googlegroups.com, trix@redhat.com
Subject: Re: [syzbot] general protection fault in PageHeadHuge
Date: Mon, 3 Oct 2022 11:24:21 -0400 [thread overview]
Message-ID: <Yzr+pSeTPB5MS6PU@x1n> (raw)
In-Reply-To: <Yzo4BU25w7i8HrrQ@monkey>
On Sun, Oct 02, 2022 at 06:16:53PM -0700, Mike Kravetz wrote:
> On 09/30/22 23:01, Peter Xu wrote:
> > On Fri, Sep 30, 2022 at 10:47:45PM -0400, Peter Xu wrote:
> > From fe9e50551f3fdb7107315784affca4f9b1c4720f Mon Sep 17 00:00:00 2001
> > From: Peter Xu <peterx@redhat.com>
> > Date: Fri, 30 Sep 2022 22:22:44 -0400
> > Subject: [PATCH] mm/hugetlb: Fix race condition of uffd missing handling
> > Content-type: text/plain
> >
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> > mm/hugetlb.c | 36 +++++++++++++++++++++++++++++++++---
> > 1 file changed, 33 insertions(+), 3 deletions(-)
> >
> > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> > index dd29cba46e9e..5015d8aa5da4 100644
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -5557,9 +5557,39 @@ static vm_fault_t hugetlb_no_page(struct mm_struct *mm,
> > if (!page) {
> > /* Check for page in userfault range */
> > if (userfaultfd_missing(vma)) {
> > - ret = hugetlb_handle_userfault(vma, mapping, idx,
> > - flags, haddr, address,
> > - VM_UFFD_MISSING);
> > + bool same;
> > +
> > + /*
> > + * Since hugetlb_no_page() was examining pte
> > + * without pgtable lock, we need to re-test under
> > + * lock because the pte may not be stable and could
> > + * have changed from under us. Try to detect
> > + * either changed or during-changing ptes and bail
> > + * out properly.
> > + *
> > + * One example of changing pte is in-progress CoW
> > + * of private mapping, which will clear+flush pte
> > + * then reinstall the new one.
> > + *
> > + * Note that userfaultfd is actually fine with
> > + * false positives (e.g. caused by pte changed),
> > + * but not wrong logical events (e.g. caused by
> > + * reading a pte during changing). The latter can
> > + * confuse the userspace, so the strictness is very
> > + * much preferred. E.g., MISSING event should
> > + * never happen on the page after UFFDIO_COPY has
> > + * correctly installed the page and returned.
> > + */
>
> Thanks Peter!
>
> The wording and pte_same check here is better than what I proposed. I think
> that last paragraph above should go into the commit message as it describes
> user visible effects (missing event after UFFDIO_COPY has correctly installed
> the page and returned).
Will do.
>
> This seems to have existed since hugetlb userfault support was added. It just
> became exposed recently due to locking changes going into 6.1. However, I
> think it may have existed in the window after hugetlb userfault support was
> added and before current i_mmap_sema locking for pmd sharing was added.
Agreed.
> Just a long way of saying I am not sure cc stable if of much value.
Logically the change is stable material. I had worry that after uffd-wp
intergration with hugetlb it's indeed possible to trigger on the CoWs we're
encountering already, so IMO still something good to have for 5.19.
I just saw that you proposed a similar fix in 4643d67e8cb0b35 on a similar
page migration race three years ago. I'm not sure whether it also can
happen with uffd missing modes too even before uffd-wp introduced.
I think I'll first post the patch with Fixes attached without having stable
tagged, but let me know your thoughts. No worry on the backport, I can
take care of doing that and tests.
I also plan to add your co-devel tag if you're fine with it because this
patch is a collaboration effort IMO, but please let me know either here or
directly replying to the patch if it's posted if you think that's inproper
in any form.
Thanks Mike!
--
Peter Xu
next prev parent reply other threads:[~2022-10-03 15:24 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-23 16:05 [syzbot] general protection fault in PageHeadHuge syzbot
2022-09-23 21:11 ` Mike Kravetz
2022-09-23 21:37 ` Yang Shi
2022-09-24 0:14 ` Hugh Dickins
2022-09-24 0:18 ` Nathan Chancellor
2022-09-24 7:28 ` Hugh Dickins
2022-09-24 0:58 ` Peter Xu
2022-09-24 15:06 ` Peter Xu
2022-09-24 19:01 ` Mike Kravetz
2022-09-26 0:11 ` Peter Xu
2022-09-27 16:24 ` Mike Kravetz
2022-09-27 16:45 ` Peter Xu
2022-09-29 23:33 ` Mike Kravetz
2022-09-30 1:29 ` Peter Xu
2022-09-30 1:35 ` Peter Xu
2022-09-30 16:05 ` Peter Xu
2022-09-30 21:38 ` Mike Kravetz
2022-10-01 2:47 ` Peter Xu
2022-10-01 3:01 ` Peter Xu
2022-10-03 1:16 ` Mike Kravetz
2022-10-03 15:24 ` Peter Xu [this message]
2022-09-25 18:55 ` Andrew Morton
2022-09-24 21:56 ` Matthew Wilcox
2022-09-27 4:32 ` Hugh Dickins
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Yzr+pSeTPB5MS6PU@x1n \
--to=peterx@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=axelrasmussen@google.com \
--cc=hughd@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=llvm@lists.linux.dev \
--cc=mike.kravetz@oracle.com \
--cc=nathan@kernel.org \
--cc=ndesaulniers@google.com \
--cc=shy828301@gmail.com \
--cc=songmuchun@bytedance.com \
--cc=syzbot+152d76c44ba142f8992b@syzkaller.appspotmail.com \
--cc=syzkaller-bugs@googlegroups.com \
--cc=trix@redhat.com \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).