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 23B17E7717D for ; Wed, 11 Dec 2024 20:56:54 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8C0946B008C; Wed, 11 Dec 2024 15:56:53 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 870AB6B0092; Wed, 11 Dec 2024 15:56:53 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 711346B0093; Wed, 11 Dec 2024 15:56:53 -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 4F9836B008C for ; Wed, 11 Dec 2024 15:56:53 -0500 (EST) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 015341612E9 for ; Wed, 11 Dec 2024 20:56:52 +0000 (UTC) X-FDA: 82883886582.07.C4FC097 Received: from mail-pl1-f171.google.com (mail-pl1-f171.google.com [209.85.214.171]) by imf20.hostedemail.com (Postfix) with ESMTP id 4A27D1C0006 for ; Wed, 11 Dec 2024 20:56:26 +0000 (UTC) Authentication-Results: imf20.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=THE5swkI; spf=pass (imf20.hostedemail.com: domain of isaacmanjarres@google.com designates 209.85.214.171 as permitted sender) smtp.mailfrom=isaacmanjarres@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1733950586; 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=k2bEWtlbySt6ThDuoSUVHIUH+ChNKWKXsxj9RWznny0=; b=w8r8wMpbvs1ihuOIxW0rgiBqQSsHI597Vh4WGF1LNrOkS7bBADMsuxhn/bszQszGU6Qpmr cGHkkyfXtoTcjiccnaX3493/cArNRkZ+KwOa607gbv88VvNDr5VoG2aK31SIrbfsOKeXeA +qwpIElj41Wme4cpzTIOpNL0oLf7YxU= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1733950586; a=rsa-sha256; cv=none; b=6ewCp0pvdhMHCkgHbLL1dtuFIc/TOo5pxGpppvNjs8YFB49Ny/q6mVNUexNSYYe3gmoK11 Ri/K9nxpyAhUb9630LUtHqY0cPepSQXy8RC8MP0RHV34DCjjOxqCLMyedhhPBtNv5mUJZZ r4gPUIPVvPd4rSXsMdu9iPglGAqCCi0= ARC-Authentication-Results: i=1; imf20.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=THE5swkI; spf=pass (imf20.hostedemail.com: domain of isaacmanjarres@google.com designates 209.85.214.171 as permitted sender) smtp.mailfrom=isaacmanjarres@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-pl1-f171.google.com with SMTP id d9443c01a7336-215740b7fb8so4485ad.0 for ; Wed, 11 Dec 2024 12:56:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1733950610; x=1734555410; 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=k2bEWtlbySt6ThDuoSUVHIUH+ChNKWKXsxj9RWznny0=; b=THE5swkIUys4EC2g+W5qmVQ+0aKF/zWpcDzp0lKjYquFR0MJ988I/Usx0AjAQhf4c7 hgeUAxx5uP/W1aJmDeq+pZpCccz6B1W4aBcS/jhGsJrXIzv0zrKebZ7wEPI4pjWUHnHi WDfmkU4Q0k7Y7H2xHZk1cgE31QntikJTYGAlXjIjHbCoFvP66tZ9moqNKnC+LUgU+cGr IbvwSE5lwARpucn67cxN5gIDiKTBNUal48SxeojVMbPSSpGBmjHpkUfHERSw7ATc1fZh 8A1+tpe0ywTGd9DjwVLbuQmqUgOl2nJNE60206O37faGoYdaPgC7KpJ85Z+vmsr2eAas 3hbw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1733950610; x=1734555410; 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=k2bEWtlbySt6ThDuoSUVHIUH+ChNKWKXsxj9RWznny0=; b=J7xhLTuNrcx52lVzDtMs6HtpFVF//6OOZNFJ1lL4SSi79aOOw8CjjyifBo7YFlmgda mqzTLHyh59OalyvdrD/8pzJWCNy0vtnjEWoenXSwgQYthJIElDgp/ztyKyD5yUsgGBwL 6FUfoYUohteECc+duNEemS0bhOwr6kLHzRrRkMl1o1oXnB1/w57VRcrsq5j7mmzAoLHQ Tnkm0zYc7/2GSHiDLeh9lB6Mk0HNNMWE5hgZ4izcW3SdSzm09EeCOU0mTalqgP8G0shm 3x0BemNy2t7Zg0l16MY+YvhO8UDHGe2n7WhPR6smaJw0Y8jRq4AucpttLVqLwjSjw26K I7Yg== X-Forwarded-Encrypted: i=1; AJvYcCWhhGn/T8wbIKfk6Mgy0cYU7bXkW4zRu2Ar7rFKqUhxSH97z/0gXH3Q7sfDZtAKYG3Af/BdEIfufg==@kvack.org X-Gm-Message-State: AOJu0YxKqbGaXemyBf5K0p0FvH2WjfVJTjIsXlkNA9u8QUmUwAN3mLT9 TQ+wih2o/bEz6kRGjGh2wLu2POfndRXhJlG7aw2dEFdh41/GYAFD4/tliHZmtw== X-Gm-Gg: ASbGnct8f2J53mFJMU6+MUXoGr9+680VwLnzQq9OHQtEbwauqb8rtCKMIriuscrqIVk yw4Wbv14W/YK+agJQi5NFDlzpSe8EQrtVCufgY3d1CmE5KM4xg875CHPWmubcLuFmHD7A4xs0Ht cgTPHOAJrqTuFWk3ftzjkdpWwfLuAG0lREwJTnDtFXBOfdnOQO95qciAA+/4shLat2Mlfvet7Li jY0YHro/qi60FysI4oeWM9e75yxcYebU+aD5wCXzJ/4y/8Zcivcxw== X-Google-Smtp-Source: AGHT+IFUNUV8Jrq3x7wIFFEJfhzg2GCvPKk4dGIRi3a8yhYwkgT2llKFjahr6PSlf2mmjBrVxBh3yA== X-Received: by 2002:a17:902:f98f:b0:215:9327:5aed with SMTP id d9443c01a7336-2178e7378a8mr470435ad.20.1733950609623; Wed, 11 Dec 2024 12:56:49 -0800 (PST) Received: from google.com ([2620:15c:2d:3:31a7:de26:ce93:3f1b]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-215f8efa162sm107919945ad.125.2024.12.11.12.56.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 11 Dec 2024 12:56:49 -0800 (PST) Date: Wed, 11 Dec 2024 12:56:44 -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> <3a53b154-1e46-45fb-a559-65afa7a8a788@lucifer.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3a53b154-1e46-45fb-a559-65afa7a8a788@lucifer.local> X-Rspamd-Queue-Id: 4A27D1C0006 X-Rspam-User: X-Rspamd-Server: rspam07 X-Stat-Signature: ojpgum57q5exgx8dwxauioi6hd6tjr7a X-HE-Tag: 1733950586-555949 X-HE-Meta: U2FsdGVkX1+RZM/o6RhrXpTGYKWz+0BvGCaUPpnJDqQpMUkvjoebWiEaI9lmBaAeF1iMblic+6dWqWDr4Bmfp+AXk2pLe4ZCVau8K7AZcDEcSSJSEMSKyNNMwswBOt0IdFyPvic2jUKozgr2PuzOUQQ3fU22bxkW/Md5QuRqRvGflvFX7JQZABUNb0qrh7Jw1fIgntgEPlvub7K4/j1R5QKDK/Cvr6Du7P/TxBpCmvYAYlTAi/RjvEg9Y3kCgz8PfMZ/YtyyIpvJolxuHWdx1GeOMKB7pM5+oxOH0OjsRTo9Gvbk8ghxfUxUm+ecxIeUIuGDmc8E2Hsmc/CtaPam7i9VCzCHW5b2FkQPlZCTe+cnjE3R6G/oIHEown+1IdCXUo4+UNH1hnyi4cuU2DAQPTYI5Rwo+ty6N5bh09g+b0JAF+SlG95vZgvTUmjfi8/TxShU2D3XdNCewSm9FaTpCYkVWfN7XVebA+RitMQETYsN3L35CrLouLVRP1lFfV1Sb2dRN08B/uhZAyvRrQcEYpvbwgGZRkfGUd6yu/5yi5CLlkexcd3Z5tTHUipCvTCNRcU4KnXXIHboQ/CvXm+MfSwz2EJqH8e4XHz858oxIEz6+ekTtBxQBUUKivrdooJEhD+XCdV1toEPzfVK4PVSExphQpFQoRIUK8QsT68ee2QiofnOgwc7GOsise3evs5/tMHGPGZSm70P/kv2J/GIng06/hKO6sQWFDT8vBee4EGaYPhTxyRjzcWnqB7lpKOvPfIqORJ/cUshGOJpx276DxfO4iMDgkSDJz9eLAv3QiWrO3WPoXmsQj1PvGoEFQWbwrbFS1mjblwxTMZCu2RgJerBFTZ3K1itk4wDPKr3u4EG7XqCc85TPTXb+ppygfQMAa9VZMKrFTOQjZ7P0xA+Tuk7KcSPMiirVPY9rDkOyd8+wmAvHSw7jGBIl6ZZtUjZm7yl3Nv88g5rZBmvjE4 FtIaOi4a /BeDZWXsRoRAXbMlGwdbkTtYgMWEJaayRPtChDVfBse/dxrxFnHbaOsXFgSwT2XirywQnUbko/8kJh0r4ne1KwhTtTgkYTA+VobkAHOOMh/Nx85Eo11/x1UMvVM4IKYwI6SAVbfpmibwjp9paJknKyoyhhmStOMnSAd/icUrPaw5XmCxqZYANgg9uUGruKsl5ktZ37WDwn1Kj8ovDBIEEegvhLwvWxohTI+dCSBdtpx5dVoJ9WClabSo5ZLVHzu3vpqZXUt7TQANQTzqmz3kvj1qaaenfhYsaOolm1Nz3ZeoiCCXbRBfVqR6okfZm3+QEjoeshzolvvmIPIh2U1HJn0wjOmudmyJ72V5iRuw3d4dklNTMjUdQ3z5GI6x5AhN78i3ZiGLryHzxegAVWLDlEv4MHuU54xtsnd8it0HgdeBP+sJacZW23rlL/rbfTXqsdw7Q2uMy+JpEM79ZziK4kECqvBweSDqpbS9fivg28gk5Q2g3TXfcu1zPady4unXYN2Kk7TtyiJ3w0y2mS4BRtFib7B/ORx8+Y7xC X-Bogosity: Unsure, tests=bogofilter, spamicity=0.473347, 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 09:14:58PM +0000, Lorenzo Stoakes wrote: > On Fri, Dec 06, 2024 at 12:48:09PM -0800, Isaac Manjarres wrote: > > 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. > > Hm, yeah I'm not sure that's really justified, I mean what's to stop a > caller from just mapping their own memory mapping executable, copying the > data and executing? > That's a fair point. In that case, I think it makes sense to enforce the seal only when the mapping is shared. The case I'm trying to address is when a process (A) allocates a memfd that is meant to be read and written by itself and another process (B). A shares the buffer with B, but B injects code into the buffer, and compromises A such that A maps the buffer with PROT_EXEC and runs the code that B injected into it. If A used F_SEAL_FUTURE_EXEC prior to sharing the buffer, then it could reduce the attack surface on itself in this scenario. > There's also further concerns around execution restriction for instance in > memfd_add_seals(): > > /* > * SEAL_EXEC implys SEAL_WRITE, making W^X from the start. > */ > if (seals & F_SEAL_EXEC && inode->i_mode & 0111) > seals |= F_SEAL_SHRINK|F_SEAL_GROW|F_SEAL_WRITE|F_SEAL_FUTURE_WRITE; > > So you probably want to change this to include F_SEAL_FUTURE_EXEC, and note Do you mean adding a case where if F_SEAL_FUTURE_EXEC is in the seals, then we should clear the X bits of the file and use F_SEAL_EXEC as well? I don't think the case in the if condition should imply F_SEAL_FUTURE_EXEC, since the file is still executable in this case? > your proposal interacts negatively with the whole > MFD_NOEXEC_SCOPE_NOEXEC_ENFORCED mode set in vm.memfd_noeec - any system > with this set to '2' will literally not allow you to do what you want if > set to 2. > > See https://origin.kernel.org/doc/html/latest/userspace-api/mfd_noexec.html Sorry, I didn't follow how this would impact MFD_NOEXEC_SCOPE_NOEXEC_ENFORCED. Could you please clarify that? > > 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, I'm just going to post to the mailing list, this is the discussion > forum I'm making use of for this :) > > I will cc- you on my patch and definitely I'd appreciate testing thanks! > > But yeah, to be clear I'm not done with reviewing this, I need more time to > digest what you're trying to do here, but you definitely need to think > about the exec limitations. Thanks for sending out the patch. I took a look and tested it out and it definitely makes implementing this a lot nicer! Thanks, Isaac