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=-2.1 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,MAILING_LIST_MULTI, MSGID_FROM_MTA_HEADER,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 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 08D58C433E2 for ; Wed, 16 Sep 2020 10:14:58 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 3290F21D43 for ; Wed, 16 Sep 2020 10:14:57 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=amdcloud.onmicrosoft.com header.i=@amdcloud.onmicrosoft.com header.b="0oSm8Hfl" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3290F21D43 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=amd.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 8B74D6B0055; Wed, 16 Sep 2020 06:14:56 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 866C66B005A; Wed, 16 Sep 2020 06:14:56 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 72FBA6B005C; Wed, 16 Sep 2020 06:14:56 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0191.hostedemail.com [216.40.44.191]) by kanga.kvack.org (Postfix) with ESMTP id 545786B0055 for ; Wed, 16 Sep 2020 06:14:56 -0400 (EDT) Received: from smtpin28.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 11A63180AD807 for ; Wed, 16 Sep 2020 10:14:56 +0000 (UTC) X-FDA: 77268516192.28.game38_1704d3f27119 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin28.hostedemail.com (Postfix) with ESMTP id DBA656C05 for ; Wed, 16 Sep 2020 10:14:55 +0000 (UTC) X-HE-Tag: game38_1704d3f27119 X-Filterd-Recvd-Size: 20182 Received: from NAM10-MW2-obe.outbound.protection.outlook.com (mail-mw2nam10on2064.outbound.protection.outlook.com [40.107.94.64]) by imf17.hostedemail.com (Postfix) with ESMTP for ; Wed, 16 Sep 2020 10:14:54 +0000 (UTC) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Ze2rx7g3mh9/29oaT/ZlRra3HtD2JaAT3LihPzB1+fxYuTezJ096wHuF9lTRiEOQ8SV+Qx8RZjFDICnlwlr2hlE7GzClgtdVVOVpjTE9IeQZneASTUylnczgXLxEKZjS/X0HV7m8Awj9ibCKJ43SAoDrSmVOBitx3JUbuxPJi+aO6A60j7TsK8aY/zhEp9bKLasFUaxnGsncLdP7KQ14JBe3p5Liz4fDsbC5BVZPsxrNhlTIYaY2crN6pejCevGWHcEiArXnn21wUSNJcPS9qR0eSJjODTlTYPOEdk3boImVoCwHbR2u2xpLrx9gXlcObIUMt0flang3GSL1l2LIYQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=ZfeUFb6U2H6zazfyJmE49VbWw7FrghWVSS5zZqtpfyY=; b=D/FkBwRoSgmCDW02Ibc73OWCtMxy15rYTsgZOFGqitNJbQ8lqQ9XCbB4pXoC1IsLW/n4+9nDmQJEicN9dI50CJYjjAF0s4v1BcgcRzrRghB9IB+YGc8SV0F9CDRydoaTTaSziPIaeRv20TKmcpZbyh9UGBO3e7jzEB+1piY8ipK1KCUWWpBAFsBeZwgL8YRcByezC+tj1YIUgEuHdoZFtKD5NXX/L4cs+JN5KJ9b+/mft6OzQWM9Gb+uaM9UmPOTB9+WzB7LDftLaz0H/P7BBzEnYYAsLsosiSD3zEUXQl5u0QZdveY9UFw4J29LwWT+2mZ//algore2lMqvZMishQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amdcloud.onmicrosoft.com; s=selector2-amdcloud-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=ZfeUFb6U2H6zazfyJmE49VbWw7FrghWVSS5zZqtpfyY=; b=0oSm8HflAIj3Bua8Y+MyIjxJNgB9/7uUHiuvkOzaZA4Lnsjp+jE1Px7DWEQXqUhpaWCdLr3+QtYZua1oVyB+Z9FiwehQW8cM8x6VqkHDttEtwhW5Jr+KHABvVWbQWxmWYDfAps+li7GWu9L/IuyVDtvvhkkmlErkJleMvz6pPw8= Authentication-Results: kvack.org; dkim=none (message not signed) header.d=none;kvack.org; dmarc=none action=none header.from=amd.com; Received: from MN2PR12MB3775.namprd12.prod.outlook.com (2603:10b6:208:159::19) by MN2PR12MB3950.namprd12.prod.outlook.com (2603:10b6:208:16d::10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3391.11; Wed, 16 Sep 2020 10:14:51 +0000 Received: from MN2PR12MB3775.namprd12.prod.outlook.com ([fe80::f8f7:7403:1c92:3a60]) by MN2PR12MB3775.namprd12.prod.outlook.com ([fe80::f8f7:7403:1c92:3a60%6]) with mapi id 15.20.3391.014; Wed, 16 Sep 2020 10:14:50 +0000 Subject: Re: Changing vma->vm_file in dma_buf_mmap() To: Jason Gunthorpe , akpm@linux-foundation.org, sumit.semwal@linaro.org, linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org References: <20200914132920.59183-1-christian.koenig@amd.com> <40cd26ae-b855-4627-5a13-4dcea5d622f6@gmail.com> <20200914140632.GD1221970@ziepe.ca> <9302e4e0-0ff0-8b00-ada1-85feefb49e88@gmail.com> <20200916095359.GD438822@phenom.ffwll.local> From: =?UTF-8?Q?Christian_K=c3=b6nig?= Message-ID: Date: Wed, 16 Sep 2020 12:14:40 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 In-Reply-To: <20200916095359.GD438822@phenom.ffwll.local> Content-Type: multipart/alternative; boundary="------------73E1BFB6D8B8C6B7178B6E38" Content-Language: en-US X-ClientProxiedBy: AM0PR10CA0033.EURPRD10.PROD.OUTLOOK.COM (2603:10a6:20b:150::13) To MN2PR12MB3775.namprd12.prod.outlook.com (2603:10b6:208:159::19) MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from [IPv6:2a02:908:1252:fb60:be8a:bd56:1f94:86e7] (2a02:908:1252:fb60:be8a:bd56:1f94:86e7) by AM0PR10CA0033.EURPRD10.PROD.OUTLOOK.COM (2603:10a6:20b:150::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3370.19 via Frontend Transport; Wed, 16 Sep 2020 10:14:49 +0000 X-Originating-IP: [2a02:908:1252:fb60:be8a:bd56:1f94:86e7] X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: e176f4eb-77a3-42f1-c4a9-08d85a2958d2 X-MS-TrafficTypeDiagnostic: MN2PR12MB3950: X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:10000; X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: PjBM/ibW8IDJixvDZe/Ml+FEKv8U9J7S8k6XLl3csMHBXNGa3vU3QgkfXDZQ6IyewUMiUhJNQ21NvWoZtuUoecZY/xNwhyAARP/1r5nSvOooDXdRBXpNDe2eDpmDqwgyZug14RgEPxLX3jfHjdSBE3dhcIQJNAsda1gF/OviWhaIWMCEX/JL/rO0PTQ9QLbCSeshHqc51fsTXS++/2lHOLwdy3E/a1g1IuwKsAV7cLIsi8lnV1Z3UN+MJaSsJJOyNx+08oJQNE+N1lU+Q92DKoqQ7FRvfkySNHUalZV9lVGat5LZ55jGyy0LzIxKTpwewOddavJzheH4tanY35PSI8u63rfObYGCzND9LmkZMLd1lxyJuruuBZBgxLUMNjuj9/jpcGyX9CnFqXJtk0qlI7M8R8QoAyBCi6VnR/sO8IkBkg1UJOd806bRVHz8sVO+bfIjSoE1qcUKo70BE5AUzA== X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:MN2PR12MB3775.namprd12.prod.outlook.com;PTR:;CAT:NONE;SFS:(4636009)(346002)(366004)(39860400002)(396003)(136003)(376002)(316002)(6666004)(6486002)(86362001)(186003)(16526019)(478600001)(2616005)(8936002)(31686004)(31696002)(8676002)(52116002)(33964004)(36756003)(2906002)(66556008)(5660300002)(83380400001)(66476007)(166002)(66574015)(66946007)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: B5DGFzpiwBubGoRoTtebkdg/zAq1qieRIx/A95wXwN7VXcHb5pLW1x6hDlp8di5jyGHkWDO4XbYn9WoyHpiBbPdbZlIKsgJ3H3QW3NZTWFeFajtjO8LumYcRl2Emt46b66v9iPn7TRf1IRwwrPda0TO8V+7Au2WZSah0QaBthQh+GTXAmXiaciTUJXe9mRQDEENbPu6hn/WAIhndgrJmbXO42B4KQyadh4fw2yCJjnDz3BFkxl2UkF/dzs+7o1pqq3TnfubrG1rl3sCd3tINx0S5okz3/zmgM/3SgKuNdtHI4VJ5jEukLqgKR5Yljmm0gxx0AvmSnBNoQopcuwYd9mbPKxAcY5596I04S1XClf2SlYL0YoATkg/RmsOwUU6RKMrp6dYfrnMW1A+zGCneZsSW7xaMvGOsegzcTkxYn5sRe7/gnxbp5lkmMTwAkBquwflZ2lHqgmuQGlkRwT7jK8/Ac0pu4qe1fdMqE+BVsMejKfs5rDR7BYm3ISd+C07EDn0LwrHo+6rzLPA+mfphYLxeUSDbaDnVhduXqrMNq1x28qxXLmIrCrgToDQAp0mpw6i6O+2LWY1KasytFQ/Vh/QB9EOr2T3GF+mYlw2xW3fssHKH5DR7GV3EQHT/pXM3K53nt+MCzA5jCzgyz+MZQrbmqg+Um1J5Xui7GfIMTqvo5SrJI7Srs4wzcWX7dsqwygAsCdyx3vaAbbiKpkZOJQ== X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: e176f4eb-77a3-42f1-c4a9-08d85a2958d2 X-MS-Exchange-CrossTenant-AuthSource: MN2PR12MB3775.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Sep 2020 10:14:50.7669 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: YSexevSaY4GlZymo45klP9ZHRqgDq/NLf9BAdBnW9IQU46RZvF4ohNlPPufhqyD1 X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR12MB3950 X-Rspamd-Queue-Id: DBA656C05 X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam02 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: --------------73E1BFB6D8B8C6B7178B6E38 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable Am 16.09.20 um 11:53 schrieb Daniel Vetter: > On Mon, Sep 14, 2020 at 08:26:47PM +0200, Christian K=C3=B6nig wrote: >> Am 14.09.20 um 16:06 schrieb Jason Gunthorpe: >>> On Mon, Sep 14, 2020 at 03:30:47PM +0200, Christian K=C3=B6nig wrote: >>>> Am 14.09.20 um 15:29 schrieb Christian K=C3=B6nig: >>>>> Hi Andrew, >>>>> >>>>> I'm the new DMA-buf maintainer and Daniel and others came up with >>>>> patches extending the use of the dma_buf_mmap() function. >>>>> >>>>> Now this function is doing something a bit odd by changing the >>>>> vma->vm_file while installing a VMA in the mmap() system call >>> It doesn't look obviously safe as mmap_region() has an interesting mi= x >>> of file and vma->file >>> >>> Eg it calls mapping_unmap_writable() using both routes >> Thanks for the hint, going to take a look at that code tomorrow. >> >>> What about security? Is it OK that some other random file, maybe in >>> another process, is being linked to this mmap? >> Good question, I have no idea. That's why I send out this mail. >> >>>>> The background here is that DMA-buf allows device drivers to >>>>> export buffer which are then imported into another device >>>>> driver. The mmap() handler of the importing device driver then >>>>> find that the pgoff belongs to the exporting device and so >>>>> redirects the mmap() call there. >>> So the pgoff is some virtualized thing? >> Yes, absolutely. > Maybe notch more context. Conceptually the buffer objects we use to man= age > gpu memory are all stand-alone objects, internally refcounted and > everything. And if you export them as a dma-buf, then they are indeed > stand-alone file descriptors like any other. > > But within the driver, we generally need thousands of these, and that > tends to bring fd exhaustion problems with it. That's why all the priva= te > buffer objects which aren't shared with other process or other drivers = are > handles only valid for a specific fd instance of the drm chardev (each > open gets their own namespace), and only for ioctls done on that charde= v. > And for mmap we assign fake (but unique across all open fd on it) offse= ts > within the overall chardev. Hence all the pgoff mangling and re-manglin= g. > > Now for unmap_mapping_range we'd like it to find all such fake offset > aliases pointing at the one underlying buffer object: > - mmap on the dma-buf fd, at offset 0 > - mmap on the drm chardev where the buffer was originally allocated, at= some unique offset > - mmap on the drm chardev where the buffer was imported, again at some > (likely) different unique (for that chardev) offset. > > So to make unmap_mapping_range work across the entire delegation change > we'd actually need to change the vma->vma_file and pgoff twice: > - once when forwarding from the importing drm chardev to the dma-buf > - once when forwarding from the dma-buf to the exported drm chardev fak= e > offset, which (mostly for historical reasons) is considered the > canonical fake offset > > We can't really do the delegation in userspace because: > - the importer might not have access to the exporters drm chardev, it o= nly > gets the dma-buf. If we'd give it the underlying drm chardev it coul= d do > stuff like issue rendering commands, breaking the access model. > - the dma-buf fd is only used to establish the sharing, once it's impor= ted > everywhere it generally gets closed. Userspace could re-export it an= d > then call mmap on that, but feels a bit contrived. > - especially on SoC platforms this has already become uapi. It's not a = big > problem because the drivers that really need unmap_mapping_range to = work > are the big gpu drivers with discrete vram, where mappings need to b= e > invalidate when moving buffer objects in/out of vram. > > Hence why we'd like to be able to forward aliasing mappings and adjust = the > file and pgoff, while hopefully everything keeps working. I thought thi= s > would work, but Christian noticed it doesn't really. Well to be clear I'm still not sure if that works or not :) But Jason pointed me to the right piece of code. See this comment in in=20 mmap_region(): > /* ->mmap() can change vma->vm_file, but must guarantee that > * vma_link() below can deny write-access if VM_DENYWRITE is set > * and map writably if VM_SHARED is set. This usually means the > * new file must not have been exposed to user-space, yet. > */ > vma ->vm_file=20 > =3D get_f= ile=20 > (file=20 > ); > error =3D call_mmap=20 > (file=20 > , vma ); So changing vma->vm_file is allowed at least under certain circumstances. Only the "file must not have been exposed to user-space, yet" part still=20 needs double checking. Currently working on that. Regards, Christian. > > Cheers, Daniel > > >> Christian. >> >>> Jason --------------73E1BFB6D8B8C6B7178B6E38 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: quoted-printable
Am 16.09.20 um 11:53 schrieb Daniel Vetter:
On Mon, Sep 14, 2020 at 08:2=
6:47PM +0200, Christian K=C3=B6nig wrote:

Am 14.09.20 um 16:06 schri=
eb Jason Gunthorpe:

On Mon, Sep 14, 2020 at =
03:30:47PM +0200, Christian K=C3=B6nig wrote:

Am 14.09.20 um 15:29 s=
chrieb Christian K=C3=B6nig:

Hi Andrew,

I'm the new DMA-buf maintainer and Daniel and others came up with
patches extending the use of the dma_buf_mmap() function.

Now this function is doing something a bit odd by changing the
vma->vm_file while installing a VMA in the mmap() system call

It doesn't look obviousl=
y safe as mmap_region() has an interesting mix
of file and vma->file

Eg it calls mapping_unmap_writable() using both routes

Thanks for the hint, going to take a look at that code tomorrow.


What about security? Is =
it OK that some other random file, maybe in
another process, is being linked to this mmap?

Good question, I have no idea. That's why I send out this mail.


The background here =
is that DMA-buf allows device drivers to
export buffer which are then imported into another device
driver. The mmap() handler of the importing device driver then
find that the pgoff belongs to the exporting device and so
redirects the mmap() call there.

So the pgoff is some vir=
tualized thing?

Yes, absolutely.

Maybe notch more context. Conceptually the buffer objects we use to manag=
e
gpu memory are all stand-alone objects, internally refcounted and
everything. And if you export them as a dma-buf, then they are indeed
stand-alone file descriptors like any other.

But within the driver, we generally need thousands of these, and that
tends to bring fd exhaustion problems with it. That's why all the private
buffer objects which aren't shared with other process or other drivers ar=
e
handles only valid for a specific fd instance of the drm chardev (each
open gets their own namespace), and only for ioctls done on that chardev.
And for mmap we assign fake (but unique across all open fd on it) offsets
within the overall chardev. Hence all the pgoff mangling and re-mangling.

Now for unmap_mapping_range we'd like it to find all such fake offset
aliases pointing at the one underlying buffer object:
- mmap on the dma-buf fd, at offset 0
- mmap on the drm chardev where the buffer was originally allocated, at s=
ome unique offset
- mmap on the drm chardev where the buffer was imported, again at some
  (likely) different unique (for that chardev) offset.

So to make unmap_mapping_range work across the entire delegation change
we'd actually need to change the vma->vma_file and pgoff twice:
- once when forwarding from the importing drm chardev to the dma-buf
- once when forwarding from the dma-buf to the exported drm chardev fake
  offset, which (mostly for historical reasons) is considered the
  canonical fake offset

We can't really do the delegation in userspace because:
- the importer might not have access to the exporters drm chardev, it onl=
y
  gets the dma-buf. If we'd give it the underlying drm chardev it could d=
o
  stuff like issue rendering commands, breaking the access model.
- the dma-buf fd is only used to establish the sharing, once it's importe=
d
  everywhere it generally gets closed. Userspace could re-export it and
  then call mmap on that, but feels a bit contrived.
- especially on SoC platforms this has already become uapi. It's not a bi=
g
  problem because the drivers that really need unmap_mapping_range to wor=
k
  are the big gpu drivers with discrete vram, where mappings need to be
  invalidate when moving buffer objects in/out of vram.

Hence why we'd like to be able to forward aliasing mappings and adjust th=
e
file and pgoff, while hopefully everything keeps working. I thought this
would work, but Christian noticed it doesn't really.

Well to be clear I'm still not sure if that works or not :)

But Jason pointed me to the right piece of code. See this comment in in mmap_region():

		/* ->mmap() can change vma->vm_file=
, but must guarantee that
		 * vma_link() below can deny write-access if VM_DENY=
WRITE is set
		 * and map writably if VM_SHARED is set. This usuall=
y means the
		 * new file must not have been exposed to user-space=
, yet.
		 */
		vma->v=
m_file =3D get_file=
(file);
		error =3D call_mmap(file<=
/span>, vma);

So changing vma->vm_file is allowed at least under certain circumstances.

Only the "file must not have been exposed to user-space, yet&quo= t; part still needs double checking. Currently working on that.

Regards,
Christian.


Cheers, Daniel



Christian.


Jason


      

      

    

    

  


--------------73E1BFB6D8B8C6B7178B6E38--