lkml.org 
[lkml]   [2020]   [Oct]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2 09/17] mm: Add unsafe_follow_pfn
    Hi Daniel,

    On Fri, Oct 09, 2020 at 07:52:05PM +0200, Daniel Vetter wrote:
    > On Fri, Oct 9, 2020 at 2:48 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
    > > On Fri, Oct 09, 2020 at 02:37:23PM +0200, Mauro Carvalho Chehab wrote:
    > >
    > > > I'm not a mm/ expert, but, from what I understood from Daniel's patch
    > > > description is that this is unsafe *only if* __GFP_MOVABLE is used.
    > >
    > > No, it is unconditionally unsafe. The CMA movable mappings are
    > > specific VMAs that will have bad issues here, but there are other
    > > types too.
    > >
    > > The only way to do something at a VMA level is to have a list of OK
    > > VMAs, eg because they were creatd via a special mmap helper from the
    > > media subsystem.
    > >
    > > > Well, no drivers inside the media subsystem uses such flag, although
    > > > they may rely on some infrastructure that could be using it behind
    > > > the bars.
    > >
    > > It doesn't matter, nothing prevents the user from calling media APIs
    > > on mmaps it gets from other subsystems.
    >
    > I think a good first step would be to disable userptr of non struct
    > page backed storage going forward for any new hw support. Even on
    > existing drivers. dma-buf sharing has been around for long enough now
    > that this shouldn't be a problem. Unfortunately right now this doesn't
    > seem to exist, so the entire problem keeps getting perpetuated.

    On the V4L2 side, I think we should disable USERPTR for any new driver,
    period. That's what I've been recommended when reviewing patches for
    several years already. It's a deprecated API, it should be phased out,
    which starts by not allowing any new use case.

    > > > If this is the case, the proper fix seems to have a GFP_NOT_MOVABLE
    > > > flag that it would be denying the core mm code to set __GFP_MOVABLE.
    > >
    > > We can't tell from the VMA these kinds of details..
    > >
    > > It has to go the other direction, evey mmap that might be used as a
    > > userptr here has to be found and the VMA specially created to allow
    > > its use. At least that is a kernel only change, but will need people
    > > with the HW to do this work.
    >
    > I think the only reasonable way to keep this working is:
    > - add a struct dma_buf *vma_tryget_dma_buf(struct vm_area_struct *vma);
    > - add dma-buf export support to fbdev and v4l
    > - roll this out everywhere we still need it.
    >
    > Realistically this just isn't going to happen. And anything else just
    > reimplements half of dma-buf, which is kinda pointless (you need
    > minimally refcounting and some way to get at a promise of a permanent
    > sg list for dma. Plus probably the vmap for kernel cpu access.
    >
    > > > Please let address the issue on this way, instead of broken an
    > > > userspace API that it is there since 1991.
    > >
    > > It has happened before :( It took 4 years for RDMA to undo the uAPI
    > > breakage caused by a security fix for something that was a 15 years
    > > old bug.
    >
    > Yeah we have a bunch of these on the drm side too. Some of them are
    > really just "you have to upgrade userspace", and there's no real fix
    > for the security nightmare without that.

    --
    Regards,

    Laurent Pinchart

    \
     
     \ /
      Last update: 2020-10-11 01:10    [W:5.973 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site