Date:   Fri, 17 Dec 2021 20:50:29 -0400
From:   Jason Gunthorpe <jgg@nvidia.com>
To:     David Hildenbrand <david@redhat.com>
Cc:     Linus Torvalds <torvalds@linux-foundation.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Hugh Dickins <hughd@google.com>,
        David Rientjes <rientjes@google.com>,
        Shakeel Butt <shakeelb@google.com>,
        John Hubbard <jhubbard@nvidia.com>,
        Mike Kravetz <mike.kravetz@oracle.com>,
        Mike Rapoport <rppt@linux.ibm.com>,
        Yang Shi <shy828301@gmail.com>,
        "Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>,
        Matthew Wilcox <willy@infradead.org>,
        Vlastimil Babka <vbabka@suse.cz>, Jann Horn <jannh@google.com>,
        Michal Hocko <mhocko@kernel.org>,
        Nadav Amit <namit@vmware.com>, Rik van Riel <riel@surriel.com>,
        Roman Gushchin <guro@fb.com>,
        Andrea Arcangeli <aarcange@redhat.com>,
        Peter Xu <peterx@redhat.com>,
        Donald Dutile <ddutile@redhat.com>,
        Christoph Hellwig <hch@lst.de>,
        Oleg Nesterov <oleg@redhat.com>, Jan Kara <jack@suse.cz>,
        Linux-MM <linux-mm@kvack.org>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@vger.kernel.org>,
        "open list:DOCUMENTATION" <linux-doc@vger.kernel.org>
Subject: Re: [PATCH v1 06/11] mm: support GUP-triggered unsharing via
 FAULT_FLAG_UNSHARE (!hugetlb)
Message-ID: <20211218005029.GG6385@nvidia.com>
References: <20211217113049.23850-1-david@redhat.com>
 <20211217113049.23850-7-david@redhat.com>
 <CAHk-=wgL5u3XMgfUN6BOqVO0OvPx3-LEri1ju-1TW4dFhHQO4g@mail.gmail.com>
 <CAHk-=wgKft6E_EeLA1GnEXcQBA9vu8m2B-M-U7PuiNa0+9gpHA@mail.gmail.com>
 <54c492d7-ddcd-dcd0-7209-efb2847adf7c@redhat.com>
 <CAHk-=wgjOsHAXttQa=csLG10Cp2hh8Dk8CnNC3_WDpBpTzBESQ@mail.gmail.com>
 <20211217204705.GF6385@nvidia.com>
 <37dddd67-7e2d-8217-b1e2-31d79bb85693@redhat.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <37dddd67-7e2d-8217-b1e2-31d79bb85693@redhat.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?lSZ0AomWEWCHKmrU8Dxuzasz93v4pq+UDzn+NbMZ8g2Rcv78qn14bYbj/ht9?=
 =?us-ascii?Q?0nCmGShdl5ApyomvGTXQmOXGplLS/2ZoqIaAeB1cZmShOTRidTWgJYKWk9Jy?=
 =?us-ascii?Q?98fg2rrV4Uek4M2p3UCmnSbIJlHIrh4mZ18oPgVgJNwVM+CxHi4E1LyTAgXX?=
 =?us-ascii?Q?rDMQ+UnDY4ku74w8Ndor1ZS5OWJ1gPUCQtOZDJYI8B8iqsBFv+LX+FossRio?=
 =?us-ascii?Q?n7J2X79yiYH+AmuBkxVM2PcBV/+VsdPoFNpLL8CBiTbf5BYwWIISmc7f55PB?=
 =?us-ascii?Q?BR/hJ/2Ar0ZfZ0UAbj5q3jEHfZ/Uir29YchKN4R3q7UHWPWzf7jdCdQ6CfpJ?=
 =?us-ascii?Q?hdKO6XySZuxiKdWdFgA2yc65b0LiPzUzBfpV5k2NIRd9qDmMuviiSlGffIqY?=
 =?us-ascii?Q?wyMKfpQS2+E00tHUvGsUS5+yNZCWPSmZegU5mUpEuiJR4FuSSNuaL6iWqHNs?=
 =?us-ascii?Q?Wl/DgbOZ5bc5D5a9e9PC7NKi4gJf9d9KhcZu7G4G8YvhaAFNFQ6PEEvdg1NW?=
 =?us-ascii?Q?wu696sV0QLl5zK7jEOk4ihGbxcf0jNOpekcV1Pp78Ul0lbWD0xjDxBG4OTeV?=
 =?us-ascii?Q?drwnHsCY6h0yZfsSnEjnTw66BTyO32CzD4fqhwkwVSp6s/l8KpTlFgnJX0lh?=
 =?us-ascii?Q?qRoer7xMIp7zOJnR7fb2suXqG07qAHsVLWoIDSOASWy4qAJh8P+mfBz0O9sH?=
 =?us-ascii?Q?oCd601068h/Nh0tUzrzizXd+FYtDAUx0mAQAruWUBdIbEAUE6NK473a9/WP4?=
 =?us-ascii?Q?vSzvGr5OJfQ055YDCjWfAv9Q0dNd2QMlx00IL1rWiw5RuGizQjbgTk94kvJN?=
 =?us-ascii?Q?n1gXrBRlKHNyUFS03FAbx9JeplEZHoUZO45s5xq8vRS+mMrYnhqL1PF4H9b4?=
 =?us-ascii?Q?70RcA6qHrpJZHpKquVcyuQm7M+HdGWB8j09pdFjIM0nyHr0Y/53KNuw6jNn1?=
 =?us-ascii?Q?Aaa49VppcwRrRuu6Fu1etndInB/M4juX9/ZzwtHP/V6yop4mJBeyD9ctCZkx?=
 =?us-ascii?Q?S6haOh6h4U8SpWgKxNlwlQTQtbDNjQrccwDm7oMUuJMPcfWTuPVk9W3Mb4qw?=
 =?us-ascii?Q?//TAChdEwyCSrP5YixSkRfjXdhO1L6+CZ7MzPBQNcc8YAT2nlD+zNQNlge1Q?=
 =?us-ascii?Q?FyLU+Hg+Jt5t2BCG0n0wjXalmn3UN5YQ/XjkEgS6thiG+mh12pkigYOJPUxU?=
 =?us-ascii?Q?EnAtKeTFocINvgwPIXGcOXBKcLnZKVKFYJ9uFD911WzfuiHOfxsLzHf4viIX?=
 =?us-ascii?Q?Nnn+BupzmLoIy+vjp9L5GQFW6HglMzn0FAm1QATUK0uQPoCpccDVBakWWt39?=
 =?us-ascii?Q?DYfpwLAaIpPC/74LziRRCJ9tSluUsQ+yrUBcof6c9c7/R1guAmVxsS+KHU1m?=
 =?us-ascii?Q?pFYZhdq1dx6cYmfIaLL+ytKqmVbDWc2gmocJlnPsZ+pvj8Oo/KpnawaaIchZ?=
 =?us-ascii?Q?ZO8CZPP80XTU/FV4+EoQ6CQqEXLmDoUC4+eDZJynpxeinDv1GpMOWR42nVcC?=
 =?us-ascii?Q?vp/ue43IkyBf/GWfbM20FDGQ+gFsOBVf/PbtaOe4oTa282TfXtAmmS1sPF9z?=
 =?us-ascii?Q?QOfRzsVxYDdH0uMPywk=3D?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 6d99bc81-4c37-4de9-ae18-08d9c1c063d1
X-MS-Exchange-CrossTenant-AuthSource: BL0PR12MB5506.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Dec 2021 00:50:30.3921
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: /y5OMCAzSND2ZqiCkmdKu61pmQWycYsE2VzT9tstNUBjfp57CGdppGP8aRGz0h/B
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL1PR12MB5206
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Fri, Dec 17, 2021 at 10:04:11PM +0100, David Hildenbrand wrote:

> > To remind all, the GUP users, like RDMA, VFIO use
> > FOLL_FORCE|FOLL_WRITE to get a 'r/o pin' specifically because of the
> 
> I heard that statement often. Can you point me at the code?
> 
> VFIO: drivers/vfio/vfio_iommu_type1.c
> 
> vaddr_get_pfns() will end up doing a 
> pin_user_pages_remote(FOLL_LONGTERM) without 
> FOLL_FORCE|FOLL_WRITE.

> Is that added automatically internally?

No, it is just that VFIO is broken in this regard. AFAIK VFIO users
rarely use the read-only mode and haven't noticed this bug yet.

You can search for FOLL_FORCE and see the drivers that do it this way:

drivers/misc/habanalabs/common/memory.c:                                 FOLL_FORCE | FOLL_WRITE | FOLL_LONGTERM,
drivers/infiniband/core/umem.c:         gup_flags |= FOLL_FORCE;
drivers/media/v4l2-core/videobuf-dma-sg.c:      unsigned int flags = FOLL_FORCE;
drivers/media/common/videobuf2/frame_vector.c:                            FOLL_FORCE | FOLL_WRITE | FOLL_LONGTERM,

[etc]

No doubt there are others that do it right and wrong, this is badly
documented and misunderstood.

> But this series really just wants to fix the security issue as "part 1".
> Without any more breakages.

Sure, I'm just trying to understand why this example was brought up.

Jason