Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 17 Apr 2023 11:15:10 -0300
From:   Jason Gunthorpe <jgg@nvidia.com>
To:     Lorenzo Stoakes <lstoakes@gmail.com>
Cc:     linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        Andrew Morton <akpm@linux-foundation.org>,
        Matthew Wilcox <willy@infradead.org>,
        David Hildenbrand <david@redhat.com>,
        Jens Axboe <axboe@kernel.dk>,
        Pavel Begunkov <asml.silence@gmail.com>,
        io-uring@vger.kernel.org
Subject: Re: [PATCH 5/7] io_uring: rsrc: use FOLL_SAME_FILE on
 pin_user_pages()
Message-ID: <ZD1UbgeoeNFEvv9/@nvidia.com>
References: <cover.1681508038.git.lstoakes@gmail.com>
 <17357dec04b32593b71e4fdf3c30a346020acf98.1681508038.git.lstoakes@gmail.com>
 <ZD1CAvXee5E5456e@nvidia.com>
 <b1f125c8-05ec-4b41-9b3d-165bf7694e5a@lucifer.local>
 <ZD1I8XlSBIYET9A+@nvidia.com>
 <34959b70-6270-46cf-94c5-d6da12b0c62d@lucifer.local>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <34959b70-6270-46cf-94c5-d6da12b0c62d@lucifer.local>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?mSPY23IUhBa3uivgGO0EkNfMl+Izz8sLcy7zAdzokUK3PeKYH7Lp2OoKM2rL?=
 =?us-ascii?Q?qqtecas0VldkNttDG//PCy/Djw4VDFhkYm/kGOu9kq30wLmkM0YqS4f69Bb9?=
 =?us-ascii?Q?rgrQGiNZfKHf4RNYy8RjBU3FpHtwSy/GL0PvoQI3MtE9SlC1fSe+AF66lnZk?=
 =?us-ascii?Q?0mgpf57P2ZDfdlWO52G4ENasnmmfLjzFE5wpFNURFHGndwXDHHyczsY8ZuiP?=
 =?us-ascii?Q?k5djl+VXkhUe7qRsn4wUcJGPplzlWPao5f4Mes256OBJQH0eGPU752GOMJf3?=
 =?us-ascii?Q?5EsH0wEoM0hsCxIZXtN+boyA9fecFGsr12F6JKjlXzdD1h8I3s0Dgvz5zH5a?=
 =?us-ascii?Q?UID6qw/AdkyZpn5SFgN7+83m33RmfBJBdq4wnZx1pmXKwbUz8UhCX5FuvY/s?=
 =?us-ascii?Q?3rIM5sj5qnNsZlDwhyL1L/4wAkrHFDCzK/loAy3+dDhBCyqo2CtrWTdgMyP1?=
 =?us-ascii?Q?/Sg3iLC5Dprm4L9q+SPodAhlHxPBZsX7nESmtBkfzLJrVQ/XOcBw/v7c9gnu?=
 =?us-ascii?Q?Py6dbbBaFSDYT1z+yFhCtnp3Im0p4cCBFcIgnRf884Cpi7iIXcYmTx7sSIQQ?=
 =?us-ascii?Q?4YDZbWsy/9H3z2N58CrFk3jO8oFNFdsBTk91vNBlsqSkM7j+29AZnax8YVHb?=
 =?us-ascii?Q?/8HF8kXfUUDYQCmYfCZW1Siye3ONsX8wwkjcAtH5UsfxgbxfsuJsd4tnobnp?=
 =?us-ascii?Q?KIYwKpn0POLxOV/DMBbpRzthGMPc+o/xoaR8wE7j68sbGcEXB4STJVSfD4oa?=
 =?us-ascii?Q?eOWgseuEZmpuBoOslf/49jDdNb4b/fclxgMrsyIn+vgfU0gg5I1DnRkZXIv/?=
 =?us-ascii?Q?9GZvFN8iGAmQhLRVh7jK1p33yyFkh4tq/YPxINvv0EA0BqJxUDxHy7UMxisa?=
 =?us-ascii?Q?8BSea6vl02fyMWo2+bmcL3Y5xi6tUPI2o/KXS9qfSSHEtjY2w8BQL5CYS1AB?=
 =?us-ascii?Q?tqxs7imJJ/x2zx6pEE/7OE+yGkTs7ZaXZ4GwDuYYXhmlqQ7GBjqSribtSQNk?=
 =?us-ascii?Q?aqPgOJ2EE536JFhXR+mSyy4f1+MCRnsvlsPGyYC1akdWQ7ESSdSVCk4hrNI+?=
 =?us-ascii?Q?e0JIFMvOEsNaUXK7esyqV/2hvUI1p4F1tZtWx4+sItdDgiujmkNoX0mn1i8g?=
 =?us-ascii?Q?pyc2jPdYVXEJknoBDVDnicxFfzNmha+ASE6j1f7qc/gqKHwFHPbuFEVqn5mZ?=
 =?us-ascii?Q?1hBFVEAd824Q+8oYV6tDCrwSw++l4GCypbYm6KUt1A8pAP2deb8cYn6jkZi5?=
 =?us-ascii?Q?n+1JqgLXWbyPNPhsd9odMJdFSv9ZstbhTzusQtoGESS0T1pehapIs1x12ZBS?=
 =?us-ascii?Q?+e7sEdZF7DDXnfDUXS4W+nTXXZqr+6q8dcxC/5oKyKeiqj/bUmgmEdAkeWtk?=
 =?us-ascii?Q?bPZ/opV9SBLA4+UScyK/1bkOKaVn9eIl7GZwjUsx7sx7E6mxYEQ5Y4jx2rzd?=
 =?us-ascii?Q?bTozhJoy8ZOzY5R9+INiP4tVu1vwFj5dOdJxhIYu1ke1FjYMMQElQEBl2Wkj?=
 =?us-ascii?Q?SpBqep5mdTK050BxpT8xi0Ii/DsNH88CglPNrgYbNkrV1ijXexb0K2yZduat?=
 =?us-ascii?Q?hqTgpnsAWKmlbCn8k9l2BkM8XF/+ozbfKWjzR3km?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7f2dd7b8-baef-4a8f-7150-08db3f4e2835
X-MS-Exchange-CrossTenant-AuthSource: LV2PR12MB5869.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Apr 2023 14:15:11.9383
 (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: KeroLZLKlk1i4/TAMlgmn1cHiRE/9qzkk+kHfD1rig4Yn9IgDPlb7gUapWyg5Oxx
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB5213
X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Mon, Apr 17, 2023 at 03:00:16PM +0100, Lorenzo Stoakes wrote:
> On Mon, Apr 17, 2023 at 10:26:09AM -0300, Jason Gunthorpe wrote:
> > On Mon, Apr 17, 2023 at 02:19:16PM +0100, Lorenzo Stoakes wrote:
> >
> > > > I'd rather see something like FOLL_ALLOW_BROKEN_FILE_MAPPINGS than
> > > > io_uring open coding this kind of stuff.
> > > >
> > >
> > > How would the semantics of this work? What is broken? It is a little
> > > frustrating that we have FOLL_ANON but hugetlb as an outlying case, adding
> > > FOLL_ANON_OR_HUGETLB was another consideration...
> >
> > It says "historically this user has accepted file backed pages and we
> > we think there may actually be users doing that, so don't break the
> > uABI"
> 
> Having written a bunch here I suddenly realised that you probably mean for
> this flag to NOT be applied to the io_uring code and thus have it enforce
> the 'anonymous or hugetlb' check by default?

Yes

> So you mean to disallow file-backed page pinning as a whole unless this
> flag is specified? 

Yes

> For FOLL_GET I can see that access to the underlying
> data is dangerous as the memory may get reclaimed or migrated, but surely
> DMA-pinned memory (as is the case here) is safe?

No, it is all broken, read-only access is safe.

We are trying to get a point where pin access will interact properly
with the filesystem, but it isn't done yet.

> Or is this a product more so of some kernel process accessing file-backed
> pages for a file system which expects write-notify semantics and doesn't
> get them in this case, which could indeed be horribly broken.

Yes, broadly

> I am definitely in favour of cutting things down if possible, and very much
> prefer the use of uaccess if we are able to do so rather than GUP.
> 
> I do feel that GUP should be focused purely on pinning memory rather than
> manipulating it (whether read or write) so I agree with this sentiment.

Yes, someone needs to be brave enough to go and try to adjust these
old places :)

I see in the git history this was added to solve CVE-2018-1120 - eg
FUSE can hold off fault-in indefinitely. So the flag is really badly
misnamed - it is "FOLL_DONT_BLOCK_ON_USERSPACE" and anon memory is a
simple, but overly narrow, way to get that property.

If it is changed to use kthread_use_mm() it needs a VMA based check
for the same idea.

Jason