From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E1005C46466 for ; Fri, 2 Oct 2020 18:17:02 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 6B24020758 for ; Fri, 2 Oct 2020 18:17:02 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=ffwll.ch header.i=@ffwll.ch header.b="UroveKdE" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6B24020758 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ffwll.ch Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 8C5CE900004; Fri, 2 Oct 2020 14:17:01 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 84F39900002; Fri, 2 Oct 2020 14:17:01 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 717F3900004; Fri, 2 Oct 2020 14:17:01 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0112.hostedemail.com [216.40.44.112]) by kanga.kvack.org (Postfix) with ESMTP id 39FBD900002 for ; Fri, 2 Oct 2020 14:17:01 -0400 (EDT) Received: from smtpin19.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id C98FF3630 for ; Fri, 2 Oct 2020 18:17:00 +0000 (UTC) X-FDA: 77327791800.19.play24_5416951271a6 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin19.hostedemail.com (Postfix) with ESMTP id 99D3A1AD1B9 for ; Fri, 2 Oct 2020 18:17:00 +0000 (UTC) X-HE-Tag: play24_5416951271a6 X-Filterd-Recvd-Size: 6290 Received: from mail-ot1-f68.google.com (mail-ot1-f68.google.com [209.85.210.68]) by imf21.hostedemail.com (Postfix) with ESMTP for ; Fri, 2 Oct 2020 18:17:00 +0000 (UTC) Received: by mail-ot1-f68.google.com with SMTP id 60so2266446otw.3 for ; Fri, 02 Oct 2020 11:16:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=3HxwxxjOyUFsHnvZd90so4n7HtMcvrp+2NDYk+teChw=; b=UroveKdEgKPvYhDgIRFBP6hGaOMHl0KhWjPmr7h+me9Xj+hsNqVEe+KzxfQ5R752mW Q39uBXtohsZOj34gGmK/hanyDQGQMTFvEwg1txwSCZatYOYvpHfVCUEfTc1tX+5qw+e4 lhr2S+yleRha4sKjZin5W1q02Y1XO7J4/uNO0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=3HxwxxjOyUFsHnvZd90so4n7HtMcvrp+2NDYk+teChw=; b=azxvmdA9R99hfKu9zi6bw4fSE6lC83yeu4yaecRX6rJzFXyqxmz0N5knDJfZ45rr4f hmitgdoNOE3WEwTqbH7KItzM2oFp3swvSp0LWsjkfUT938EiUCc7ZZd2l3a9CkS7wQRa 4+eNnHnHoqnmjSBGPzsjCkIVvpNjLJveQKKN+faZ+w1dt7bZRVkwT6VqxXrQslGNdDYy lOac7OF845dTyOuQNSudr6HBCe/wbEAPxv74UZBNgf66C4GmVGqNOU7HB5IebzPJLdM8 kCkH29qraC5Oq+7ytQ3kC90vh7Uy1xIsqpLAjEFlcnkGqsjcaQdE3qavBZQvvKAkJVrS Q7ig== X-Gm-Message-State: AOAM5339Sh97JwfTFT6FsRzSQtlIGa985C/i1TTRSmXoxS72Cj4RVoHL TZWIlc+TlI9m4hYCkGDAOHWIwAQfK7ujSzfMlFpYaQ== X-Google-Smtp-Source: ABdhPJz/iLpxd0MKJvGez6uG++167T4Nd3ZEU24lIiueGBImWGdkODsxyKl+FpR0pn01MP2E0GHPOCfnbU7WfP4NbAY= X-Received: by 2002:a9d:4b99:: with SMTP id k25mr2747483otf.281.1601662619231; Fri, 02 Oct 2020 11:16:59 -0700 (PDT) MIME-Version: 1.0 References: <20201002175303.390363-1-daniel.vetter@ffwll.ch> <20201002175303.390363-2-daniel.vetter@ffwll.ch> <20201002180603.GL9916@ziepe.ca> In-Reply-To: <20201002180603.GL9916@ziepe.ca> From: Daniel Vetter Date: Fri, 2 Oct 2020 20:16:48 +0200 Message-ID: Subject: Re: [PATCH 2/2] mm/frame-vec: use FOLL_LONGTERM To: Jason Gunthorpe Cc: DRI Development , LKML , Daniel Vetter , Andrew Morton , John Hubbard , =?UTF-8?B?SsOpcsO0bWUgR2xpc3Nl?= , Jan Kara , Dan Williams , Linux MM , Linux ARM , linux-samsung-soc , "open list:DMA BUFFER SHARING FRAMEWORK" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Fri, Oct 2, 2020 at 8:06 PM Jason Gunthorpe wrote: > On Fri, Oct 02, 2020 at 07:53:03PM +0200, Daniel Vetter wrote: > > For $reasons I've stumbled over this code and I'm not sure the change > > to the new gup functions in 55a650c35fea ("mm/gup: frame_vector: > > convert get_user_pages() --> pin_user_pages()") was entirely correct. > > > > This here is used for long term buffers (not just quick I/O) like > > RDMA, and John notes this in his patch. But I thought the rule for > > these is that they need to add FOLL_LONGTERM, which John's patch > > didn't do. > > > > There is already a dax specific check (added in b7f0554a56f2 ("mm: > > fail get_vaddr_frames() for filesystem-dax mappings")), so this seems > > like the prudent thing to do. > > > > Signed-off-by: Daniel Vetter > > Cc: Andrew Morton > > Cc: John Hubbard > > Cc: J=C3=A9r=C3=B4me Glisse > > Cc: Jan Kara > > Cc: Dan Williams > > Cc: linux-mm@kvack.org > > Cc: linux-arm-kernel@lists.infradead.org > > Cc: linux-samsung-soc@vger.kernel.org > > Cc: linux-media@vger.kernel.org > > Hi all, > > > > I stumbled over this and figured typing this patch can't hurt. Really > > just to maybe learn a few things about how gup/pup is supposed to be > > used (we have a bit of that in drivers/gpu), this here isn't really > > ralated to anything I'm doing. > > FOLL_FORCE is a pretty big clue it should be FOLL_LONGTERM, IMHO Since you're here ... I've noticed that ib sets FOLL_FORCE when the ib verb access mode indicates possible writes. I'm not really clear on why FOLL_WRITE isn't enough any why you need to be able to write through a vma that's write protected currently. > > I'm also wondering whether the explicit dax check should be removed, > > since FOLL_LONGTERM should take care of that already. > > Yep! Confirms the above! > > This get_vaddr_frames() thing looks impossible to use properly. How on > earth does a driver guarentee > > "If @start belongs to VM_IO | VM_PFNMAP vma, we don't touch page > structures and the caller must make sure pfns aren't reused for > anything else while he is using them." > > The only possible way to do that is if the driver restricts the VMAs > to ones it owns and interacts with the vm_private data to refcount > something. > > Since every driver does this wrong anything that uses this is creating > terrifying security issues. > > IMHO this whole API should be deleted :( Yeah that part I just tried to conveniently ignore. I guess this dates back to a time when ioremaps where at best fixed, and there wasn't anything like a gpu driver dynamically managing vram around, resulting in random entirely unrelated things possibly being mapped to that set of pfns. The underlying follow_pfn is also used in other places within drivers/media, so this doesn't seem to be an accident, but actually intentional. I guess minimally we'd need a VM_PFNMAP flag for dynamically manged drivers like modern drm gpu drivers, to make sure follow_pfn doesn't follow these? -Daniel --=20 Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch