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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7B60DC25B45 for ; Mon, 23 Oct 2023 18:57:09 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DBF476B014B; Mon, 23 Oct 2023 14:57:08 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D6EBF6B014C; Mon, 23 Oct 2023 14:57:08 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C36676B014D; Mon, 23 Oct 2023 14:57:08 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id B3C536B014B for ; Mon, 23 Oct 2023 14:57:08 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 646F21CB656 for ; Mon, 23 Oct 2023 18:57:08 +0000 (UTC) X-FDA: 81377633736.22.1D07100 Received: from mail-yb1-f170.google.com (mail-yb1-f170.google.com [209.85.219.170]) by imf19.hostedemail.com (Postfix) with ESMTP id 8F0291A0012 for ; Mon, 23 Oct 2023 18:57:05 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="xREuj0/i"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf19.hostedemail.com: domain of surenb@google.com designates 209.85.219.170 as permitted sender) smtp.mailfrom=surenb@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1698087425; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=2yqqvVSxSrZk5/pIhCtZC81tg5RVxwkjwG9586NW/1Y=; b=ALUnmWatyw6Yv1oHo36QB/qAZdQmq/lmUnUVmbRNmQwuAQVTpsDAIKK7Ls5gpvu6T640LL n3a8xFVcLn3P3ru26rYj//x81H6WaGj8okKqIzLyLO9IrhzLqXdpjRbwAKMEsc++nLcO/I Pl/VEGMlIS+UPas384o6Y8D21OO6m9U= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="xREuj0/i"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf19.hostedemail.com: domain of surenb@google.com designates 209.85.219.170 as permitted sender) smtp.mailfrom=surenb@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1698087425; a=rsa-sha256; cv=none; b=dFrr+sP0lhpBoAwY1Mg/CPHWJ6cK7NX3XkIVOIxxC6bNs8YBcO4fS/GMU6VaOywiZkT5u/ Bv6T5w6gcHaTuGd3/Y4s6o8RkK3G67aWUt1eHd9wA0qpPIeB5xUusUCPUVde+BOYpMi822 +7xLmtph99BvGFBYr/zrBvqK6F75WHk= Received: by mail-yb1-f170.google.com with SMTP id 3f1490d57ef6-d9a3d737d66so2808468276.2 for ; Mon, 23 Oct 2023 11:57:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1698087424; x=1698692224; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=2yqqvVSxSrZk5/pIhCtZC81tg5RVxwkjwG9586NW/1Y=; b=xREuj0/ibZlYEOaVM5lvI6VdwgUmkpk1N0QUIH/FKFp+Q/pnxu4AjAOO6b43RGoJLm C54eibwI58+hSh9VFysCDSMVcM1Qxm7SJoQhiPPbo9L5XQNI4jY6pa7SCiRf2cPC6M0C QYAthxG9hf8Vkn1alj13nDLCunCs374Hjbg9glnIpN5PwuTTSL6IGtTyPzH1NR/Y4sIf w0J5nGT6u51SAm8i9JzpLTSKhCJNWVUNBtAixN4ylydvplP3ULjRa0xx/CJ5U0uqMbKh rJKNzgvJc4Ruo1cZZshMugPS6+83VhUyb04ObhOV4jVmJ7tP8GGa84kttS3t1wNRSQuf tV3w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698087424; x=1698692224; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=2yqqvVSxSrZk5/pIhCtZC81tg5RVxwkjwG9586NW/1Y=; b=UCs/WqNGTGSq5CBvQdqwNtY2C2tOQHZTwxqM7KmB9NvQzkXhEGI+LLdLdiuOeNXGXY S/8gj36macYFOpJhN/CFQM0iADJCs+y3c16m6FAiNlCKvt9fyNbhw1ufMGgs3apEhjdL I83nzDdmNctZqO7lt9xH9WH4+7YGCzsBqWv1PnlHnI65/xyLVhUrfK9kQhfef1YxJQk5 1nac/rEGec2Rth+IWz5QZkYAMp3XmREinAiazD0GP0pUtb4taxGxq6LOuchX3O8/63NX ieCnh/lJWfNLNDinrRbJXQvsQhfogzVxHkeeU4cCLl2dwQLJEDqbZcZ45vZ/kjRyiZjG OOvA== X-Gm-Message-State: AOJu0YyF3zUz+46ovFJ5lqvsV+S9VWl5p277us+9TnKeFialHGXZC7+z 4hpctgT2oQ5m0MvMXofAxREF9qwOL8NjfSoo+rn/HA== X-Google-Smtp-Source: AGHT+IHNQUEfjaARExOLomnsfHsumATRv6sH0Qq1Lhf5GhWbo8qoEifZvXPrlpQp8hcXTGjjpDeRLsUE+mhmHgPPud4= X-Received: by 2002:a25:410e:0:b0:d9c:a3b8:f39d with SMTP id o14-20020a25410e000000b00d9ca3b8f39dmr8025930yba.65.1698087424379; Mon, 23 Oct 2023 11:57:04 -0700 (PDT) MIME-Version: 1.0 References: <20231009064230.2952396-1-surenb@google.com> <20231009064230.2952396-3-surenb@google.com> <721366d0-7909-45c9-ae49-f652c8369b9d@redhat.com> In-Reply-To: <721366d0-7909-45c9-ae49-f652c8369b9d@redhat.com> From: Suren Baghdasaryan Date: Mon, 23 Oct 2023 11:56:50 -0700 Message-ID: Subject: Re: [PATCH v3 2/3] userfaultfd: UFFDIO_MOVE uABI To: David Hildenbrand Cc: akpm@linux-foundation.org, viro@zeniv.linux.org.uk, brauner@kernel.org, shuah@kernel.org, aarcange@redhat.com, lokeshgidra@google.com, peterx@redhat.com, hughd@google.com, mhocko@suse.com, axelrasmussen@google.com, rppt@kernel.org, willy@infradead.org, Liam.Howlett@oracle.com, jannh@google.com, zhangpeng362@huawei.com, bgeffon@google.com, kaleshsingh@google.com, ngeoffray@google.com, jdduke@google.com, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, kernel-team@android.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 8F0291A0012 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: d8bk96eyjmcxpwqukou8d5ka6fh8r6uc X-HE-Tag: 1698087425-933583 X-HE-Meta: U2FsdGVkX1/QmtYTAWjBhSk8MJX3ULOsnzVGIy8qkMMMib2hmhjDZElPx31g71ovFJRRZDPYdWrAuQColcHosOadB2diwMb/gcFVDiQyHqAd4tj4BTkQD3bXWK5Kl7U2KwPt2klW2Hb0AT48NH6npxq0JO4PZAX1iZOKTMeE9h2Rnlka7ILOC5coSM3oKe+lFVYwsNMQNEQ6FN/u1jNxEwRF6rWbRCKC8Z5KdSkKNQXq8Q8EDKexmePk2qXAyBybr7cpXnPRM9gDzkgcyR56ouatBHm4GKdDqIVDEYko7Fms/y/H9S/bafVVAA7/2etKMnsZLtQa8aikP+3JysTFiYgbFF5sof69Fy4I8jdI4kCph2OIGrkzzdDmXeRcNaEDF0asOT1emhxAOsSG1TlWTH3BMnRCl4Mca/zL3qhuk0JuDCnOE+27Jb3S+YIuHcP/Xeu54ChzIzMBFavGpkI9NYUDJdAmwJSCQSAZo9OUSRkTpANngQo09lZw9ZLSHpNtP/1GNNjaSNaEFrwvuvrxLAWhH+EWEbxVaFLagfQC1qFhQNyb5oe5Z/qLG0wGRraJxmz9TyMkI2L0C64cDlRWF1TcD9iVGLR33+qFmmqGXhsJ6WhtvzqvDe0Mte7HhaFmfvIXH2kEKc2wYxhYkIZSiJZ0+W961R0k7HSbXLOrgvDilDnHPMQwa2uMOPngfVgIcQahYIKFoKmGEv7o0pF5Xdc8CCvkKVC7hoF5mMlcqZ8Ad2kFtn9T5n5iO4qeAe+31zq+HCHurJbEyF3ZN7ZZuhUnWeyxVFi3jtBvCj/LhvVCXx6a/twOeKgETkAZ9yLJu54OuutEAJAA7CP0ENEiEe6ZAOY1+bVHtg4cK6/TMilFLPAzSm0vKgslheOlriOvfD5m+jV92lC1ULjGF3swuxVfRsGECYkgX+uXh3CORJ0qiHRPIzhjdmCXkbs4J3Nf/vkKOJ/MHZo2iOK0N/+ /kJATt5w MBtSJjO/bgruz3Z6JZlBzwlpY1rxoQIFlUxv5VVWMesNVHewEjcroAMnorQASiJr2MRDP6nPAYq+S4qXVUbfHYI2yDpRAV8zN7WvUWLdr+mQLAPlEOQ6KVCHIXaUOMVoPCUbgmQ3pnvL3fE8sq/Yl8UD+wWRyG4k01WOmDmMhsTN+WVDZpUh6hhuVQoDtlsR++DaGYzBrUJ0Kdcj5HQMG4OAUDsBMjhvl6vhuaWRlz1C+/jZ5bpMWflqKwmsoRwd08noS6arH4/ZMdCA+mSpUbc0zGjNHJYsa+xigYpCKsK92MZ7wJZQNjwHuuVUfyL8X7IkUEDg7LGSJD1vav4OasqfPkAAE+d/UHW0y 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: List-Subscribe: List-Unsubscribe: On Mon, Oct 23, 2023 at 5:29=E2=80=AFAM David Hildenbrand wrote: > > Focusing on validate_remap_areas(): > > > + > > +static int validate_remap_areas(struct vm_area_struct *src_vma, > > + struct vm_area_struct *dst_vma) > > +{ > > + /* Only allow remapping if both have the same access and protecti= on */ > > + if ((src_vma->vm_flags & VM_ACCESS_FLAGS) !=3D (dst_vma->vm_flags= & VM_ACCESS_FLAGS) || > > + pgprot_val(src_vma->vm_page_prot) !=3D pgprot_val(dst_vma->vm= _page_prot)) > > + return -EINVAL; > > Makes sense. I do wonder about pkey and friends and if we even have to > so anything special. I don't see anything special done for mremap. Do you have something in mind= ? > > > + > > + /* Only allow remapping if both are mlocked or both aren't */ > > + if ((src_vma->vm_flags & VM_LOCKED) !=3D (dst_vma->vm_flags & VM_= LOCKED)) > > + return -EINVAL; > > + > > + if (!(src_vma->vm_flags & VM_WRITE) || !(dst_vma->vm_flags & VM_W= RITE)) > > + return -EINVAL; > > Why does one of both need VM_WRITE? If one really needs it, then the > destination (where we're moving stuff to). As you noticed later, both should have VM_WRITE. > > > + > > + /* > > + * Be strict and only allow remap_pages if either the src or > > + * dst range is registered in the userfaultfd to prevent > > + * userland errors going unnoticed. As far as the VM > > + * consistency is concerned, it would be perfectly safe to > > + * remove this check, but there's no useful usage for > > + * remap_pages ouside of userfaultfd registered ranges. This > > + * is after all why it is an ioctl belonging to the > > + * userfaultfd and not a syscall. > > I think the last sentence is the important bit and the comment can > likely be reduced. Ok, I'll look into shortening it. > > > + * > > + * Allow both vmas to be registered in the userfaultfd, just > > + * in case somebody finds a way to make such a case useful. > > + * Normally only one of the two vmas would be registered in > > + * the userfaultfd. > > Should we just check the destination? That makes most sense to me, > because with uffd we are resolving uffd-events. And just like > copy/zeropage we want to resolve a page fault ("userfault") of a > non-present page on the destination. I think that makes sense. Not sure why the original implementation needed the check for source too. Seems unnecessary. > > > > + */ > > + if (!dst_vma->vm_userfaultfd_ctx.ctx && > > + !src_vma->vm_userfaultfd_ctx.ctx) > > + return -EINVAL; > > > > > + > > + /* > > + * FIXME: only allow remapping across anonymous vmas, > > + * tmpfs should be added. > > + */ > > + if (!vma_is_anonymous(src_vma) || !vma_is_anonymous(dst_vma)) > > + return -EINVAL; > > Why a FIXME here? Just drop the comment completely or replace it with > "We only allow to remap anonymous folios accross anonymous VMAs". Will do. I guess Andrea had plans to cover tmpfs as well. > > > + > > + /* > > + * Ensure the dst_vma has a anon_vma or this page > > + * would get a NULL anon_vma when moved in the > > + * dst_vma. > > + */ > > + if (unlikely(anon_vma_prepare(dst_vma))) > > + return -ENOMEM; > > Makes sense. > > > + > > + return 0; > > +} > > Thanks, Suren. > -- > Cheers, > > David / dhildenb > > -- > To unsubscribe from this group and stop receiving emails from it, send an= email to kernel-team+unsubscribe@android.com. >