On Tue, Aug 31, 2021 at 9:36 AM Kirill A. Shutemov <kirill@xxxxxxxxxxxxx> wrote:
>
> On Mon, Aug 30, 2021 at 06:55:02PM +0800, Yafang Shao wrote:
> > On Mon, Aug 30, 2021 at 6:08 PM Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:
> > >
> > > On Mon, Aug 30, 2021 at 05:12:32PM +0800, Yafang Shao wrote:
> > > > > Which is not a valid way to call get_user_pages. What we need to do is
> > > > > to reject that case.
> > > >
> > > > Do you mean below change ?
> > >
> > > Sory of. I think once touching this we should do a few more cleanups
> > > including making many of the flags private to gup.c. I'll try to find
> > > some time to post a more complete series.
> >
> > JFYI, below test case can also hit the bug I reported above.
>
> How does the bug manifests with the test case? I don't see any crash with
> it in my setup.
>
> Or do you mean you can hit __get_user_pages() with FOLL_MLOCK, but without
> FOLL_POPULATE?
>
Right, that is what I meant.
> My guess is that you have wrong expectation from GUP: it will return a
> number of pages it advanced in the mapping, not number of present pages
> there. For your case it means that the array of pages can have gaps and
> it's okay.
>
Right, I misunderstood __get_user_pages(). Thanks for the explanation.
> Fill the array with zeros before calling GUP and check if the entry is
> non-NULL before dereferencing it.
>
Sure.
> >
> > #define _GNU_SOURCE
> > #include <stdio.h>
> > #include <sys/mman.h>
> >
> > #define LEN 4096
> >
> > int main()
> > {
> > char *addr;
> > int ret;
> >
> > addr = mmap(NULL, LEN, PROT_READ|PROT_WRITE, MAP_PRIVATE |
> > MAP_ANON , -1, 0);
> > if (addr == MAP_FAILED) {
> > perror("mmap");
> > return ret;
> > }
> >
> > /*
> > * MLOCK_ONFAULT will hit below if condition.
> > * if ((*flags & (FOLL_POPULATE | FOLL_MLOCK)) == FOLL_MLOCK)
> > * return -ENOENT;
> > */
> > ret = mlock2(addr, LEN, MLOCK_ONFAULT);
> > // ret = mlock2(addr, LEN, 0);
> > if (ret < 0) {
> > perror("mlock2");
> > return ret;
> > }
> >
> > return 0;
> > }
> >
> > --
> > Thanks
> > Yafang
> >
>
> --
> Kirill A. Shutemov
--
Thanks
Yafang
