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 337E5E7717B for ; Sat, 7 Dec 2024 19:22:59 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B79EE6B00B4; Sat, 7 Dec 2024 14:22:58 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B2A756B00B6; Sat, 7 Dec 2024 14:22:58 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A19846B00B8; Sat, 7 Dec 2024 14:22:58 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 825996B00B4 for ; Sat, 7 Dec 2024 14:22:58 -0500 (EST) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 10E8F161CFD for ; Sat, 7 Dec 2024 19:22:58 +0000 (UTC) X-FDA: 82869135090.24.0B20188 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf14.hostedemail.com (Postfix) with ESMTP id 05BAF100003 for ; Sat, 7 Dec 2024 19:22:35 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=mPYS1xaf; spf=pass (imf14.hostedemail.com: domain of sj@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=sj@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1733599357; 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-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=itPxPBIgbRZmJaCkkSwR/mFIcxPFe2yPTh21I4U9tbA=; b=l+R9tfh8jPDWi9Paw5fHJEax54NRh9o6s47+bwg27wPP/R+IWq8czFf4bR5+jvGVmf5vxv 6f1OiKNMZpqzk5UOaWSpsvwbIAktgEgduPwYV9Xg4moxWMG+8i1Pqii9QgrsJzI6UuG4/R wCBaLzw4pPBumgdbZwsyktky/vdPV2U= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1733599357; a=rsa-sha256; cv=none; b=6Ge4dybDGpx/7DJy8CbBGh4wUpK0JV1+STHwHluL3XvSpcwc2au0zzKMxUiQKax0xm6qZ4 ymSwojKxDWE6CdQjXxnVyo6mLfL71Cu2IPwXklY7df3q3Km+q1FqW4hnKw9FXq3QxGDx8G fkD70ikH/N5fl0pEp29ofmQBArAr1LY= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=mPYS1xaf; spf=pass (imf14.hostedemail.com: domain of sj@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=sj@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id A84305C1628; Sat, 7 Dec 2024 19:22:12 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id C4072C4CECD; Sat, 7 Dec 2024 19:22:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1733599374; bh=PgqB4r1QP9LHy8XRk/NMlDcLiT/kOPHlGNLDtInk86U=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=mPYS1xafs8YRN9tYC6W9AslH3uRQ8wA0Aa8ZoNriap2xhTJVCSslEPTM8YrcQNDOi f1timoUSz3aMTubTBsvW1daQeium6DzRabh1xhoYltZAoQodPAba933mBwhMKYfsDv p0LXnQOX888c+9sdGkhrO4vzdlzLnK25A/4PaPMWfVV+Yjd8mNk9zMUSP7dDYLmR7X DHRNeuuE9mHCKHy0vb+NqA4rfohX9wOJr+9q0s1ns4d5TWFR6eEF46GVXpdEoWQPfj R7CxjDgVWHoTwS47APd4BzSFoS8C4Y1WTL7ZX6ax1uClk0VUTsHw2G/yPIJ2ypuuvi RtM1uCkmYRYbg== From: SeongJae Park To: Lorenzo Stoakes Cc: SeongJae Park , "Liam R . Howlett" , Muchun Song , Vlastimil Babka , Jann Horn , Hugh Dickins , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Isaac Manjarres , Kalesh Singh Subject: Re: [PATCH] mm: perform all memfd seal checks in a single place Date: Sat, 7 Dec 2024 11:22:51 -0800 Message-Id: <20241207192251.204814-1-sj@kernel.org> X-Mailer: git-send-email 2.39.5 In-Reply-To: <20241206212846.210835-1-lorenzo.stoakes@oracle.com> References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 05BAF100003 X-Rspam-User: X-Rspamd-Server: rspam07 X-Stat-Signature: mgm4pnmhk7znt6fj83pewkmb37br3tqc X-HE-Tag: 1733599355-698524 X-HE-Meta: U2FsdGVkX19XKBbvMqxOjkQtsTja0K4CCM+vWZZRh+FtvWncj/IJyu0fDaFa74e8wXqsDR4obmkkS2Leg85UVHTHRM+Z/Mp3fo+sHllMAjYFl6kK8gipEbopCBhy61wIDXLk2VDhng5c/LgyzjDjudMKWPU3kq/nuIzw8MFXi90PI7nJeDIP2H8syqwfostOtRs79O+xY8IBoMwgVFDaPaa8capz/dsGFnCPRbTiBWd+T+szB7vIPTn731OuBvkCvefkjoxbjSqkOmDE8u1aC6I4eRXrQmGtFotrjlQX91m4sFjsnp4vvYrEZJA7Ebutq0WNjA5Mgi8HGK6MpZ62xVm2rRShBlyWUOQT73/5cQeVDdliG+jb3WSpVTTl+tbw78yzx4I3voPdYq8YdvPxZXswsyUTL0ekSFCzO3OxOEvejKt8aGFR4cDLNLeAY32mxKDfd1IBDRZJ2+dZVN7oi1IDcxtdo1u5e4/8qH7wDVJN79FCbWyQhACXo2rwt1vdMslVHj/bm0jzXvcLGyj9Al6C2lLIvj59FYF5JGJVhDYQWDKHr71X2LYYs687vkEiGeBdbAdFJoCDtrzDgVvPdcvmLi10nLvWMK7XIEAawKPRM9lBCAOywxb6CT4L7FIHqWBAnbD1TnFI4lRuRm7rAzyKMynU7oPrjPULMGNVBNe7cYoG4X2W31P4m1/th5J4yD2LVsflxf4n6lWGHg1+QEpBhmrPH0NoVQrfpKk9IeZYvMdm4CVMEXBN0qpDAXBSQDEQEW//lu1qravP14Bd3nZRw0hA6jW+MX3LO4eb2Alg/p39nHgKNkhvGLQFa0Sphu15UaEdtxTkYyV2NSaskEm6Yx73QYBri0WZbXNOwb/wzPPg2tb+dCxUrr2l3XCKqA8ZzpEGRr42w1/ZAfDELzb/Sbx8dpmLZbTba2uwKV+2lcs2LpfuL9MznSohIJ4kEL9GITsEHksTfkHit6b oNu2YuxQ viVbpr0P8SzBIZRiIOc9+v/vDeZCeIbr60ym0KWfnM7U9GyVFyaLOn0SCLHeWY6dqOp4jQ7iVNhBfYEaASq5V2PyU1klb4SY4CFDqtGrIBubVQp97C7ppYIbECLEZn4AFnBE5XsaTs4/YLMh2H7EkNShA+uICCz26a7qGokGCr+AfNsGoxqNjvyv3hW4w59uEUkXJCDbH0TCkx4DHcBcoBjcKBot0nSXfC8N8dbIDUU6VouA= 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: Hi Lorenzo, On Fri, 6 Dec 2024 21:28:46 +0000 Lorenzo Stoakes wrote: > We no longer actually need to perform these checks in the f_op->mmap() hook > any longer. > > We already moved the operation which clears VM_MAYWRITE on a read-only > mapping of a write-sealed memfd in order to work around the restrictions > imposed by commit 5de195060b2e ("mm: resolve faulty mmap_region() error > path behaviour"). > > There is no reason for us not to simply go ahead and additionally check to > see if any pre-existing seals are in place here rather than defer this to > the f_op->mmap() hook. > > By doing this we remove more logic from shmem_mmap() which doesn't belong > there, as well as doing the same for hugetlbfs_file_mmap(). We also remove > dubious shared logic in mm.h which simply does not belong there either. > > It makes sense to do these checks at the earliest opportunity, we know > these are shmem (or hugetlbfs) mappings whose relevant VMA flags will not > change from the invoking do_mmap() so there is simply no need to wait. > > This also means the implementation of further memfd seal flags can be done > within mm/memfd.c and also have the opportunity to modify VMA flags as > necessary early in the mapping logic. > > Signed-off-by: Lorenzo Stoakes > --- > fs/hugetlbfs/inode.c | 5 ---- > include/linux/memfd.h | 22 ++++++++--------- > include/linux/mm.h | 55 ------------------------------------------- > mm/memfd.c | 44 +++++++++++++++++++++++++++++++++- > mm/mmap.c | 12 +++++++--- > mm/shmem.c | 6 ----- > 6 files changed, 62 insertions(+), 82 deletions(-) > [...] > diff --git a/include/linux/memfd.h b/include/linux/memfd.h > index d437e3070850..d53408b0bd31 100644 > --- a/include/linux/memfd.h > +++ b/include/linux/memfd.h > @@ -7,7 +7,14 @@ > #ifdef CONFIG_MEMFD_CREATE > extern long memfd_fcntl(struct file *file, unsigned int cmd, unsigned int arg); > struct folio *memfd_alloc_folio(struct file *memfd, pgoff_t idx); > -unsigned int *memfd_file_seals_ptr(struct file *file); > +/* > + * Check for any existing seals on mmap, return an error if access is denied due > + * to sealing, or 0 otherwise. > + * > + * We also update VMA flags if appropriate by manipulating the VMA flags pointed > + * to by vm_flags_ptr. > + */ > +int memfd_check_seals_mmap(struct file *file, unsigned long *vm_flags_ptr); > #else > static inline long memfd_fcntl(struct file *f, unsigned int c, unsigned int a) > { > @@ -17,19 +24,10 @@ static inline struct folio *memfd_alloc_folio(struct file *memfd, pgoff_t idx) > { > return ERR_PTR(-EINVAL); > } > - > -static inline unsigned int *memfd_file_seals_ptr(struct file *file) > +int memfd_check_seals_mmap(struct file *file, unsigned long *vm_flags) Shouldn't we set this function as 'static inline'? Otherwise, build fails when CONFIG_MEMFD_CREATE is not defined, like below. ld: mm/mmap.o: in function `memfd_check_seals_mmap': mmap.c:(.text+0x...): multiple definition of `memfd_check_seals_mmap'; mm/gup.o:gup.c:(.text+0x...): first defined here ld: mm/secretmem.o: in function `memfd_check_seals_mmap': secretmem.c:(.text+0x...): multiple definition of `memfd_check_seals_mmap'; mm/gup.o:gup.c:(.text+0x...): first defined here Also a trivial nit. The second argument's name (vm_flags) is different from that for CONFIG_MEMFD_CREATE=y (vm_flags_ptr). > { > - return NULL; > + return 0; > } > #endif > > -/* Retrieve memfd seals associated with the file, if any. */ > -static inline unsigned int memfd_file_seals(struct file *file) > -{ > - unsigned int *sealsp = memfd_file_seals_ptr(file); > - > - return sealsp ? *sealsp : 0; > -} > - > #endif /* __LINUX_MEMFD_H */ Thanks, SJ [...]