From: Mike Kravetz <mike.kravetz@oracle.com>
To: David Hildenbrand <david@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Jason Gunthorpe <jgg@nvidia.com>,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
Peter Xu <peterx@redhat.com>, John Hubbard <jhubbard@nvidia.com>,
syzbot+f0b97304ef90f0d0b1dc@syzkaller.appspotmail.com
Subject: Re: [PATCH v1] mm/gup: disallow FOLL_FORCE|FOLL_WRITE on hugetlb mappings
Date: Tue, 22 Nov 2022 09:41:07 -0800 [thread overview]
Message-ID: <Y30Js24EqCncvqO/@monkey> (raw)
In-Reply-To: <4b926dc7-b8ec-e363-b9d8-35c0f2c897f6@redhat.com>
On 11/22/22 10:05, David Hildenbrand wrote:
> On 21.11.22 22:33, Andrew Morton wrote:
> > On Mon, 21 Nov 2022 09:05:43 +0100 David Hildenbrand <david@redhat.com> wrote:
> >
> > > > > MikeK do you have test cases?
> > > >
> > > > Sorry, I do not have any test cases.
> > > >
> > > > I can ask one of our product groups about their usage. But, that would
> > > > certainly not be a comprehensive view.
> > >
> > > With
> > >
> > > https://lkml.kernel.org/r/20221116102659.70287-1-david@redhat.com
> > >
> > > on it's way, the RDMA concern should be gone, hopefully.
> > >
> > > @Andrew, can you queue this one? Thanks.
> >
> > This is all a little tricky.
> >
> > It's not good that 6.0 and earlier permit unprivileged userspace to
> > trigger a WARN. But we cannot backport this fix into earlier kernels
> > because it requires the series "mm/gup: remove FOLL_FORCE usage from
> > drivers (reliable R/O long-term pinning)".
> >
> > Is it possible to come up with a fix for 6.1 and earlier which won't
> > break RDMA?
>
> Let's recap:
Thanks!
>
> (1) Nobody so far reported a RDMA regression, it was all pure
> speculation. The only report we saw was via ptrace when fuzzing
> syscalls.
>
> (2) To trigger it, one would need a hugetlb MAP_PRIVATE mappings without
> PROT_WRITE. For example:
>
> mmap(0, SIZE, PROT_READ,
> MAP_PRIVATE|MAP_ANON|MAP_HUGETLB|MAP_HUGE_2MB, -1, 0)
> or
> mmap(0, SIZE, PROT_READ, MAP_PRIVATE, hugetlbfd, 0)
>
> While that's certainly valid, it's not the common use case with
> hugetlb pages.
FWIW, I did check with our product teams and they do not knowingly make use
of private mappings without write. Of course, that is only a small and
limited sample size.
RDMA to shared hugetlb mappings is the common case.
>
> (3) Before 1d8d14641fd9 (< v6.0), it "worked by accident" but was wrong:
> pages would get mapped writable into page tables, even though we did
> not have VM_WRITE. FOLL_FORCE support is essentially absent but not
> fenced properly.
>
> (4) With 1d8d14641fd9 (v6.0 + v6.1-rc), it results in a warning instead.
>
> (5) This patch silences the warning.
>
>
> Ways forward are:
>
> (1) Implement FOLL_FORCE for hugetlb and backport that. Fixes the
> warning in 6.0 and wrong behavior before that. The functionality,
> however, might not be required in 6.2 at all anymore: the last
> remaining use case would be ptrace (which, again, we don't have
> actual users reporting breakages).
>
> (2) Use this patch and backport it into 6.0/6.1 to fix the warning. RDMA
> will be handled properly in 6.2 via reliable long-term pinnings.
I am OK with this approach.
--
Mike Kravetz
>
> (3) Use this patch and backport it into 6.0/6.1 to fix the warning.
> Further, backport the reliable long-term pinning changes into
> 6.0/6.1 if there are user reports.
>
> (4) On user report regarding RDMA in 6.0 and 6.1, revert the sanity
> check that triggers the warning and restore previous (wrong)
> behavior.
>
>
> To summarize, the benefit of (1) would be to have ptrace on hugetlb COW
> mappings working. As stated, I'd like to minimize FOLL_FORCE implementations
> if there are no legacy users because FOLL_FORCE has a proven record of
> security issues. Further, backports to < 6.0 might not be straight forward.
>
> I'd suggest (2), but I'm happy to hear other opinions.
>
> --
> Thanks,
>
> David / dhildenb
>
next prev parent reply other threads:[~2022-11-22 17:42 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-31 15:25 [PATCH v1] mm/gup: disallow FOLL_FORCE|FOLL_WRITE on hugetlb mappings David Hildenbrand
2022-10-31 16:14 ` Jason Gunthorpe
2022-10-31 22:13 ` Mike Kravetz
2022-11-21 8:05 ` David Hildenbrand
2022-11-21 21:33 ` Andrew Morton
2022-11-22 9:05 ` David Hildenbrand
2022-11-22 17:41 ` Mike Kravetz [this message]
2022-11-22 17:59 ` Jason Gunthorpe
2022-11-22 23:03 ` Andrew Morton
2022-11-22 23:48 ` Mike Kravetz
2022-11-23 8:40 ` David Hildenbrand
2022-11-02 9:14 ` David Hildenbrand
2022-11-02 11:38 ` Jason Gunthorpe
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=Y30Js24EqCncvqO/@monkey \
--to=mike.kravetz@oracle.com \
--cc=akpm@linux-foundation.org \
--cc=david@redhat.com \
--cc=jgg@nvidia.com \
--cc=jhubbard@nvidia.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=peterx@redhat.com \
--cc=syzbot+f0b97304ef90f0d0b1dc@syzkaller.appspotmail.com \
/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).