Messages in this thread | | | Subject | Re: [PATCH v5 05/15] mm/frame-vector: Use FOLL_LONGTERM | From | John Hubbard <> | Date | Sat, 31 Oct 2020 22:22:11 -0700 |
| |
On 10/31/20 7:45 AM, Daniel Vetter wrote: > On Sat, Oct 31, 2020 at 3:55 AM John Hubbard <jhubbard@nvidia.com> wrote: >> On 10/30/20 3:08 AM, Daniel Vetter wrote: ... >> By removing this check from this location, and changing from >> pin_user_pages_locked() to pin_user_pages_fast(), I *think* we end up >> losing the check entirely. Is that intended? If so it could use a comment >> somewhere to explain why. > > Yeah this wasn't intentional. I think I needed to drop the _locked > version to prep for FOLL_LONGTERM, and figured _fast is always better. > But I didn't realize that _fast doesn't have the vma checks, gup.c got > me a bit confused.
Actually, I thought that the change to _fast was a very nice touch, btw.
> > I'll remedy this in all the patches where this applies (because a > VM_IO | VM_PFNMAP can point at struct page backed memory, and that > exact use-case is what we want to stop with the unsafe_follow_pfn work > since it wreaks things like cma or security). > > Aside: I do wonder whether the lack for that check isn't a problem. > VM_IO | VM_PFNMAP generally means driver managed, which means the > driver isn't going to consult the page pin count or anything like that > (at least not necessarily) when revoking or moving that memory, since > we're assuming it's totally under driver control. So if pup_fast can > get into such a mapping, we might have a problem. > -Daniel >
Yes. I don't know why that check is missing from the _fast path. Probably just an oversight, seeing as how it's in the slow path. Maybe the appropriate response here is to add a separate patch that adds the check.
I wonder if I'm overlooking something, but it certainly seems correct to do that.
thanks, -- John Hubbard NVIDIA
| |