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 A265CC25B48 for ; Tue, 24 Oct 2023 14:37:09 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3791C6B02B1; Tue, 24 Oct 2023 10:37:09 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 328FA6B02B2; Tue, 24 Oct 2023 10:37:09 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1F0686B02B3; Tue, 24 Oct 2023 10:37:09 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 1165F6B02B1 for ; Tue, 24 Oct 2023 10:37:09 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 737371CB30F for ; Tue, 24 Oct 2023 14:37:08 +0000 (UTC) X-FDA: 81380607336.07.E185E04 Received: from mail-ot1-f49.google.com (mail-ot1-f49.google.com [209.85.210.49]) by imf25.hostedemail.com (Postfix) with ESMTP id AED96A0008 for ; Tue, 24 Oct 2023 14:37:06 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="ekCY/p/T"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf25.hostedemail.com: domain of surenb@google.com designates 209.85.210.49 as permitted sender) smtp.mailfrom=surenb@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1698158226; a=rsa-sha256; cv=none; b=dWiQONU3dkeVYcACpUdCkCX3W2TLUyCqk+erUZCIeRtPORoS/lGIRs7BQDhvQugdkYfINX 4rucXxz4e9EGQp+yNaA7fIHjQ8xekOFReR5qzXoGV8efnEaMoUrbK6r1ac9uuHYy7U4u3H K2/D6x3TUJwxhrpHzzxgT8XMU09zm+A= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="ekCY/p/T"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf25.hostedemail.com: domain of surenb@google.com designates 209.85.210.49 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=1698158226; 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=AAOhUkJjCv880R1NKQbAjINSlXvyzmgC43sxJnCAkZ0=; b=6E1THDwerCqYLRuPy11SVHjrp1ia9HzqzmpxpB0GQ3F1P+s1nxEWxJSg2ih/+5ldN6FNOj WXRkCXtjekiV1sgwtyMTgi79qsNa62zWaET5dt9SO2s5fV08cDupVjE3pwY64cGas7r7Yt dLzc/rF1m2eyYF1rf3egdD/FS7BPqnw= Received: by mail-ot1-f49.google.com with SMTP id 46e09a7af769-6c4cbab83aaso2998492a34.1 for ; Tue, 24 Oct 2023 07:37:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1698158225; x=1698763025; 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=AAOhUkJjCv880R1NKQbAjINSlXvyzmgC43sxJnCAkZ0=; b=ekCY/p/TX230y+i3u+2PTD1P3/47ohYZVM6an1MxA57KYLLBuAO6K+T0ysZXamYq+K SrOceapZDUlnAdjV4414tKVsaBGDqjEM1sY5L9TECxfoHCQwFze7555OueVORhNiN6dl AHiGnarcoB+xg0ZBdwClI1zTYYybpNdb0a6Y1QRozBpK+3QUb5zUhC1WQ3U6D+t2yUTP 53lM2UkOWmQzpBidQJRZL4D2RtTqeRBewwBmnWVz5r6AVUs/4sBQqr9HLuuCTYxJro8y UUyl1Mi/Sm48fEexhYMVKV+dvM3LmRIyY3IE7+7SEFUN7Gxj+J8h9RaRE9GywuIWsD7z b5/w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698158225; x=1698763025; 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=AAOhUkJjCv880R1NKQbAjINSlXvyzmgC43sxJnCAkZ0=; b=IVQEZfdFMnTPWpgdH78ZtMP9GheYU950nGnzD58NkPRU3ms9Pb4D5pu6ZygbDeAffn Fa7Vqk9lp93QRv1VrJkP2nXt5Ul7Sjy6TIEWpQ9X9HMmCmguNqbiRYu4ErHMs9qGRt70 3hLteCEBTEIug/Gf++bkWjL2XeMXM0QPRfECxYGuG9jKpjxKu3Ifwm75e6lKDUI8vaFr Lf57vACdz5ldOwdTp6fkLcUNw8Fruk86UsiIt8M+WiSLXjpMuvcKXeW6cIUeGJ23b3hk gh6f0ctYUvbdKg0b1lqr07zu5XaWxKM13bIKqrKe0rEy1WdVmdYHwkX9ZYMagxa80MxL AoSw== X-Gm-Message-State: AOJu0Yw8S/d0Q7QJ/tJs1AHEmgABpEQTqQ9QBRtTYyyXdbEQ+3FykqMD HtVQd6WC4Mv/BprhRypdlogU1EcVo/XXiCxMwjJheg== X-Google-Smtp-Source: AGHT+IGmaL1lCubVBE3q3NWkBv6nBQrZ0wRsy6ngThBW4I3ExrtLI9tz2AGRjsobSL7vHMdWw5sje4QO7Pqt6rn0QgM= X-Received: by 2002:a05:6830:1e30:b0:6af:95f9:7adc with SMTP id t16-20020a0568301e3000b006af95f97adcmr12051128otr.14.1698158225267; Tue, 24 Oct 2023 07:37:05 -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> <356a8b2e-1f70-45dd-b2f7-6c0b6b87b53b@redhat.com> In-Reply-To: <356a8b2e-1f70-45dd-b2f7-6c0b6b87b53b@redhat.com> From: Suren Baghdasaryan Date: Tue, 24 Oct 2023 07:36:51 -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-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: AED96A0008 X-Stat-Signature: kw8ybobddoi1pgq57fm6m87kr1e6fh7q X-HE-Tag: 1698158226-819390 X-HE-Meta: U2FsdGVkX18QjR0+1WrAkAov4m92KjTx+M4VP5hwfdjDnG612HaeiKE1WDqxalXCrm7nXEiNuC4XvErGj/tcFCFebSbOiIvg9iO5RDM9gwkkabbK1VNJApLm4Rzln6i/qTV3HUa0wxHw4NEf5mkv3GOfG84YKl/PIbELFZX2ZSnKifAXVJwvRhAyiWTIYBIucz2w3df6TXyo1kSpuCqmA9gLVfis+7BDM0sf5MJww+nBxg8V6z28o356eG1aQlvKrh9s/QjKFyeBLptaraUHzz3nrFNUBJLKT0Z95hV0OlDU/H0eQf5GfaWifXpOkSxkhStsYjyYI3KJ+bbMJ0M9KWnZ2/T3+Nq1UoB7XhAImF2pU+g1q9DHMvqJx0n8sAo2ctcjgsZ4V0Wg8BGH6PwmrodKl8XzrlQzGKJr9mBoazA5hfat+Y1mQswpvUvzVtfnnccywkDsXjDLm/qMovPjxV0UrWIf7+9E75QLXAX/Fu/x64R1lRuwccDpomkRAYD8OX1iAo0XSnENhemlxi4vdFxstNDeEMylp/HBZA8Y4wUotOXNQIcY1P+kKbb3d//+GHh5wCzaK7rQZXb57WTPTer+XNfur8PH8H3uhqdQ8u8Nbb7Fi9mYYUjql3IGm/3T2fDFDUhCjkELzPQqDL5oCAfaYU9aSEXfhT0bzjDdu10HUnzd8b/LtFypWi6XCe/tWZilqQ8k946CPBgkThTFCJFAX5akbIYCJHyOHWXFCAjqXVCM4n6s3MEEABOr3Tr5LarUWRH3qFrK8Dq9YAmoClQt+LKCSEElW2OMiDF3WaYxKJFV6Vs2dS3DZWhkgNhM+fyTDBWVQ7XrsHN7tr3OTmjOYBEib1VaoEYDx+hseBhu1k0O6cF3h0H4uaurqftu4EaTGIH4mLhyOXrJvzZo3Nr9BQAkCS3O10+hLztpPFWPBThSTurLXrct3Jqz0x/RvcMnASTmh2xttZ6FXZt Gs3qPcA9 W6wmoD8WE/YeMuh9cpyI8xJYz5bmsyhRM3sovkfuCMrLhO+mgbShjkU0ZYhnTFivHkiLB73q/uhQ5qR8JK51Gw6ctAr6Qs4V35nMe/JWdVcLU8gnFIR/bXf8WCgruShT+wzy6nk6th729ftCLDJZk5qMlWnqU//sfbZtOnKTu+fbnCmnZuQ4uCvqbgVWLqlXkfP+Z8qhFgDjd4GfzVe8Jwui5JuPvRm9D0GT748x//GKySdtijDhf+OT/Ga9XM0lZchl2Sm+pC3nLhu0zNYMVafE01gcsqaDhJ+PtiUKQRWoNnQOjIzD658NQUly7Lgfoz1zGKFLt+3iMGK/WBs+wZ28O7M9Dg6rdvfigh0vnhkPlz5vw2bnfgSVumcKYYLhjiPxV 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 Tue, Oct 24, 2023 at 7:27=E2=80=AFAM David Hildenbrand wrote: > > On 23.10.23 20:56, Suren Baghdasaryan wrote: > > 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 protec= tion */ > >>> + if ((src_vma->vm_flags & VM_ACCESS_FLAGS) !=3D (dst_vma->vm_fla= gs & 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? > > Nothing concrete, not a pkey expert. But as there is indeed nothing > pkey-special in the VMA, there is nothing we can really check for and/or > adjust. > > So let's assume this is fine. Sounds good until someone tells us otherwise. > > >> > >>> + > >>> + /* Only allow remapping if both are mlocked or both aren't */ > >>> + if ((src_vma->vm_flags & VM_LOCKED) !=3D (dst_vma->vm_flags & V= M_LOCKED)) > >>> + return -EINVAL; > >>> + > >>> + if (!(src_vma->vm_flags & VM_WRITE) || !(dst_vma->vm_flags & VM= _WRITE)) > >>> + 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. > > Can you comment why? Just a simplification for now? Would be good to add > that comment in the code as well. Yeah, I thought to move a page both areas should be writable since we are technically modifying both by this operation. > > /* For now, we keep it simple and only move between writable VMAs. */ Ack. Will add. > > >>> + */ > >>> + 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. > > > That is rather future work (or what's to fix here?) and better > documented in the cover letter. Ack. > > Having thought about VMA checks, I do wonder if we want to just block > some VM_ flags right at the beginning (VM_IO,VM_PFNMAP,VM_HUGETLB,...). > That might be covered by some other checks here implicitly, but I'm not > 100% sure if that's always the case. An explicit list as in > vma_ksm_compatible() might be clearer. > > Further, I wonder if we have to block VM_SHADOW_STACK; we certainly > don't want to let users modify the shadow stack by moving modified > target pages into place. But this might already be covered by earlier > checks (vm_page_prot? but I didn't look up with which setting we ended > up in the upstream version). Good point. I'll check if existing checks already cover these and if not will add them. Thanks, Suren. > > Cc'ing Rick: see "validate_remap_areas()" in [1] > > [1] https://lkml.kernel.org/r/20231009064230.2952396-3-surenb@google.com > > > -- > Cheers, > > David / dhildenb >