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 83ACCE77173 for ; Fri, 6 Dec 2024 20:48:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BF15D6B02E9; Fri, 6 Dec 2024 15:48:18 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B7A2B6B02EA; Fri, 6 Dec 2024 15:48:18 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9CDA06B02EB; Fri, 6 Dec 2024 15:48:18 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 7AA2F6B02E9 for ; Fri, 6 Dec 2024 15:48:18 -0500 (EST) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id AF265A0607 for ; Fri, 6 Dec 2024 20:48:17 +0000 (UTC) X-FDA: 82865720616.20.8ECA844 Received: from mail-pl1-f182.google.com (mail-pl1-f182.google.com [209.85.214.182]) by imf29.hostedemail.com (Postfix) with ESMTP id 512DE120005 for ; Fri, 6 Dec 2024 20:47:51 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=yUzCjrY8; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf29.hostedemail.com: domain of isaacmanjarres@google.com designates 209.85.214.182 as permitted sender) smtp.mailfrom=isaacmanjarres@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1733518087; a=rsa-sha256; cv=none; b=w8QgKeTHdAZUzBkneWdbEoifa2YcoMeCLDFjfM9eHuudkxwSUZ7gSAysLF5X3ftGijQvI0 T6XsmiDGJ2Dwf1AgIyI9Sj9xLy1MytSvDLtX1qTwdN+oVxTFC6yPpDoQcwne4ktgLTUdpi DGhWvyNk/cyXn0mbVe2FZ7GgqbkQM+Y= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=yUzCjrY8; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf29.hostedemail.com: domain of isaacmanjarres@google.com designates 209.85.214.182 as permitted sender) smtp.mailfrom=isaacmanjarres@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1733518087; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=0fOhJQgbWZQ80jDiX7GHnsujPHU3MBTZQhNqik9LVNw=; b=SBFvqbPnx1E1ZXz/miFaDhYEeGesuYfvQ5sxOFyPwlSq2gG0y6nlLmmApaDBlZbWpvJgpv yG7l5kv0tec2ujgQ4P2teCCH59axjySH3nhC6ERQR5/kWwbGw4/8uEyziAPINtPs0vF2/3 guGyGIHcJ8uFG4QQrVAwryqn4SMaJdk= Received: by mail-pl1-f182.google.com with SMTP id d9443c01a7336-21625b4f978so28365ad.0 for ; Fri, 06 Dec 2024 12:48:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1733518094; x=1734122894; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=0fOhJQgbWZQ80jDiX7GHnsujPHU3MBTZQhNqik9LVNw=; b=yUzCjrY8/XAgfWi0Za9PM+DtxbZ24w89QOHKGGc2Mf1nZUw+asj/CLEVWXgR0vfbKf GXwIE4CGP3kIHFXotMs9Waeq4xX4oBHW9lAEtkqxfUHHtc4RNygLd9Nx/5iC3WHt4aKV cNewYd4dKzMuLbX5FjEGawh1nt/DnYn2ZgcmrupqofNtsUEfIVjehjUGY5p2dsXZ0kTH HL/m7jQJbxkFiptLIqb2sxDRj0OLfNyfjbLx2WsFwfoPmh200TK7m/vbdO2YiI1FiJMC OEovjSIR9eybqxys3E5IdkJa4eQtuG6lYUa5RGd88NU+gKd0Tt/jjdlpW/ZzAPc0bWUb zrXQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1733518094; x=1734122894; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=0fOhJQgbWZQ80jDiX7GHnsujPHU3MBTZQhNqik9LVNw=; b=cuP5i1+IqqkDtXenVuVtPRQ68Li2RD4wUnp+iG9Siv4E8uJkdtJcYeh6l1oP6Yrx+F VpAqfUPdGj5hcQR4sbW8mBC/g/M2g8K29eywPvkIyXPkMJbP6kkNy99Jjr77F0vUxPxw ZAJYNWCM4dx7i6R8QX5mhGSpV6EjmUJRCGkUECXsJeseVfS7PVHDO+1LCgzzlX6KIw8P Zbt1nNAsKGREQNFUyo24V4g+nDy2y65HfBEF/tj5/k52nO12FE9XkUR2Sx+QlUa/z8YD tW6NjBeFgVVcyK+JWX/dcwsb/WHWd2ZnsQYwSVTZYgA5hkkGFKNmMtS4y8PqIXKpoHO/ cDaw== X-Forwarded-Encrypted: i=1; AJvYcCUtu+uoev2LdiY/zNuI3zxvXm2/TKSMIW5RGCaj6OY+AHjFvz8OwAtfuoJap0Sw4cp6dUHI0c1Zhg==@kvack.org X-Gm-Message-State: AOJu0Yzc3m3lpDlYPfnttOQJoWbYcH2trrWcGIgZ1M05dk8Dor7xMUmk kC0X+b0f7SKpkB7bej7aF3+pMk9Mes2sMlxaQuNwh/iwvqceacGyitOxeIgdCQ== X-Gm-Gg: ASbGncsPcnKhzzEbJFhWGvb27nyt0zP4j5nEP4Ng/YHXSl6epw0nC5vEhpGTenePykU u4DdVFetQJt22stFTCDv3QsMRxVaYJO/btSg1K6KtltzRliAHoUND7vqqEGS7gbu/H0kSw3SVcW ewlhBrloA87AjV3zA3dGqLqW8LWQMetVc6XRPb02gC82sIVSduvd6HtOmFB3DFXypidFN4b2Y9p TMc3jsfXGTHqYR9NmkKmcLpj9iyipCiOIfqaxPdVisbUuZi0w== X-Google-Smtp-Source: AGHT+IGdvvFO8AqI8xisd9ogpnZo5Wa1deBjCsmxcE8SMTmX0XgZ1ut0QcYHjXAgifFdtr7D72jJVQ== X-Received: by 2002:a17:902:ef10:b0:215:b464:340d with SMTP id d9443c01a7336-2162ad9684dmr316095ad.21.1733518094377; Fri, 06 Dec 2024 12:48:14 -0800 (PST) Received: from google.com ([2620:15c:2d:3:7caa:6c4:e72a:a87d]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-7fd156e1e20sm3464231a12.32.2024.12.06.12.48.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 06 Dec 2024 12:48:13 -0800 (PST) Date: Fri, 6 Dec 2024 12:48:09 -0800 From: Isaac Manjarres To: Lorenzo Stoakes Cc: Andrew Morton , Jeff Layton , Chuck Lever , Alexander Aring , "Liam R. Howlett" , Vlastimil Babka , Jann Horn , Shuah Khan , kernel-team@android.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kselftest@vger.kernel.org, Suren Baghdasaryan , Kalesh Singh , John Stultz Subject: Re: [RFC PATCH v1 1/2] mm/memfd: Add support for F_SEAL_FUTURE_EXEC to memfd Message-ID: References: <20241206010930.3871336-1-isaacmanjarres@google.com> <20241206010930.3871336-2-isaacmanjarres@google.com> <0ff1c9d9-85f0-489e-a3f7-fa4cef5bb7e5@lucifer.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <0ff1c9d9-85f0-489e-a3f7-fa4cef5bb7e5@lucifer.local> X-Rspam-User: X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 512DE120005 X-Stat-Signature: irb8bm1x9w6ofsig1dnfnyosd9d69bpk X-HE-Tag: 1733518071-299349 X-HE-Meta: U2FsdGVkX19Y4jQhSzjGZlt4+ZWcIoi1ZBZZNjgxwvjzT703sYW71zEOGKAevuCfix8rnWkf7vyJwLoI72G2h152aDvYFW8tmZfr7A9h61iGC7x7MiHXPCwHrKhMlqtFyxiTC3weFY9eZlvW7ekgfLk1lFMzSPNmB+qbdkfXLyrYbF0dN5YiyidQkLvRpzE/5/UONc64GEYRy2JHer8b+XWa/sHU7kbhYLX/2INE9bPqrDpTkdHOf3poBhhlJ8nof8Bo4ws4Pd6RdAeyVOp6sfQxgv0zJGd4IugJqGJ1JaK0wPFUhiEmZmZ3jcGl3Sh3h+hEMxz2p7G5kuJBcOyXhQTVCLXZ23mMyEBISMmWzgqnmu7PlGiVT/yg7/JUBSVKEZfp1Q+YbIjJ7loFq+9HKmGzWh2awBzmIAm68a66zDipslxfMGPvSzw5LhZp9R2leUEBGFi00eGr3Qm6ZVQ3tlIWd6GKuQBmXGl88tbQvCy48ou4xmidFaonlkraeT1nPGiDnPiuvAk9nLmBVrzAts6+fCqGJkG9RucSKgsMsYyoEUsQVoL1vgG/6z2mEXp8PrWVioaZS3B3L1UqJ/dtksnToWk/klyoC25tLWrSK3/bx00OxRygSsIFsxLYMr5aujR/ePvhWtXRDKNpNHXFgGjq9gj+wa43uAkXu0legptFVCY1EaOoVRGxMrbTB++GZE4h/xdc7a0Ahfy+xJTsPfAYD1nM73F5vLpg/0SaCjxLrMykKKFfv4pM67IYQpeN5eiJKZeSavhyc6WInsUgMhn2cMkm8/Sk1SbtflDMA/AHkCSQK7VGI3sR0uAO5ALjxpYj9Iw1/78LjBN5tLpnDnLZf7DBp1GLXb6rX5/tiQVC4fo7AtcjOrSBeuTmFap6ezIjEQGdaErYTzqMqIuD/6asOwCDFYRD7WiD3yOYDkWFQc4yP7rGoU1BcUGIanQ7nxDz47/1wwm3DrlvGS3 9sPyEthL XIBmIVm26NfeMW/mYFCjsV6BqsvsJ6DXo46byWnUqZF1IIKNUNllnE4+SevFD05zshwUOJUfuuDmEtK/rDppABFVzxLDBbn8HP0/YjWHjwPPW2JjGrPGLdH4rAhFe/jsfQ96tSVrkZqnSlmM8hqZTnQYTlWNeJ+Xrto/e/QeTCzYSMBLMhNk1sP93egDQ8Go5QaWonG6jailxYDYH5PX7ofMok/uvmTjfTBlwp0xKv50tAdTTBRGaIXYxONNPMUYV8pU6u90zHVnHgKdmWZgCX5ao6Xy3kYWsL70Z9Yw3lvOh8NUTgknLwRQOgPc0PmqT3tTE7ipDCpW1yEW0sa8tVHUA+6HuWSIOr8/0G8uLAcUm1iwHxeulTCVhaQtpqyPg/OrI0KPpdvLwCLbWlxs6NMIoGcyAuJezAkBkKKyiiXQ5VWO4YTdzSsjkELhHCPY+Hm7jbYLrDJBCSIUXylo9Qf8C2saNRwzMhy3JduDhJDmZJcd+nnsURGKV3qITBotk/IfSrhBEd7Y2cJzIqwJtO56A1Fn6dPiLGeiM X-Bogosity: Ham, tests=bogofilter, spamicity=0.002285, 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 Fri, Dec 06, 2024 at 06:19:49PM +0000, Lorenzo Stoakes wrote: > On Thu, Dec 05, 2024 at 05:09:22PM -0800, Isaac J. Manjarres wrote: > > diff --git a/mm/mmap.c b/mm/mmap.c > > index b1b2a24ef82e..c7b96b057fda 100644 > > --- a/mm/mmap.c > > +++ b/mm/mmap.c > > @@ -375,6 +375,17 @@ unsigned long do_mmap(struct file *file, unsigned long addr, > > if (!file_mmap_ok(file, inode, pgoff, len)) > > return -EOVERFLOW; > > > > Not maybe in favour of _where_ in the logic we check this and definitely > not in expanding this do_mmap() stuff much further. > > See comment at bottom though... I have a cunning plan :) > > > + if (is_exec_sealed(seals)) { > > Are we intentionally disallowing a MAP_PRIVATE memfd's mapping's execution? > I've not tested this scenario so don't know if we somehow disallow this in > another way but note on write checks we only care about shared mappings. > > I mean one could argue that a MAP_PRIVATE situation is the same as copying > the data into an anon buffer and doing what you want with it, here you > could argue the same... > > So probably we should only care about VM_SHARED? Thanks for taking a look at this! I'd originally implemented it for just the VM_SHARED case, but after discussing it with Kalesh, I changed it to disallow executable mappings for both MAP_SHARED and MAP_PRIVATE. Our thought was that write sealing didn't apply in the MAP_PRIVATE case to support COW with MAP_PRIVATE. There's nothing similar to COW with execution, so I decided to prevent it for both cases; it also retains the same behavior as the ashmem driver. > > + /* No new executable mappings if the file is exec sealed. */ > > + if (prot & PROT_EXEC) > > Seems strange to reference a prot flag rather than vma flag, we should have > that set up by now. That makes sense. I can change this to check for VM_EXEC in v2 of this series. > > + return -EACCES; > > + /* > > + * Prevent an initially non-executable mapping from > > + * later becoming executable via mprotect(). > > + */ > > + vm_flags &= ~VM_MAYEXEC; > > + } > > + > > You know, I'm in two minds about this... I explicitly moved logic to > do_mmap() in [0] to workaround a chicken-and-egg scenario with having > accidentally undone the ability to mmap() read-only F_WRITE_SEALed > mappings, which meant I 'may as well' move the 'future proofing' clearing > of VM_MAYWRITE for F_SEAL_FUTURE_WRITE too. > > But now I feel that the use of shmem_mmap() and hugetlbfs_file_mmap() to do > _any_ of this is pretty odious in general, we may as well do it all > upfront. > > [0]:https://lore.kernel.org/all/cover.1732804776.git.lorenzo.stoakes@oracle.com/ I agree. I really like the idea of handling the future proofing and error checking in one place. It makes understanding how these seals work a lot easier, and has the added benefit of only worrying about the check once rather than having to duplicate it in both shmem_mmap() and hugetlbfs_file_mmap(). > > flags_mask = LEGACY_MAP_MASK; > > if (file->f_op->fop_flags & FOP_MMAP_SYNC) > > flags_mask |= MAP_SYNC; > > -- > > 2.47.0.338.g60cca15819-goog > > > > So actually - overall - could you hold off a bit on this until I've had a > think and can perhaps send a patch that refactors this? > > Then your patch can build on top of that one and we can handle this all in > one place and sanely :) > > Sorry you've kicked off thought processes here and that's often a dangerous > thing :P Thanks again for reviewing these patches! Happy that I was able to get the gears turning :) I'm really interested in helping with this, so is there any forum you'd like to use for collaborating on this or any way I can help? I'm also more than happy to test out any patches that you'd like! Thanks, Isaac