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 D4DBDC25B7C for ; Thu, 23 May 2024 02:33:18 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 6C0FC6B0089; Wed, 22 May 2024 22:33:18 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6707F6B008C; Wed, 22 May 2024 22:33:18 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5387C6B0092; Wed, 22 May 2024 22:33:18 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 35C366B0089 for ; Wed, 22 May 2024 22:33:18 -0400 (EDT) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id AB08016016E for ; Thu, 23 May 2024 02:33:17 +0000 (UTC) X-FDA: 82148088834.21.AFD86B8 Received: from mail-ed1-f51.google.com (mail-ed1-f51.google.com [209.85.208.51]) by imf24.hostedemail.com (Postfix) with ESMTP id D3C3218000F for ; Thu, 23 May 2024 02:33:15 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=1J6uthgr; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf24.hostedemail.com: domain of jeffxu@google.com designates 209.85.208.51 as permitted sender) smtp.mailfrom=jeffxu@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1716431596; 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=TRtZAXBVtEg5v5aAZ0fCa+gcBJjQ9sT10KYuWxMy/d8=; b=QUzCEQ4ZPfs7stod9gZHhtibe1Y77ICYuK9jqf+eRudSM2iI7AFVcYiTj21uMtWQzG0Z69 jw9nLWfb6sGdp9voYgS4ZMzlkMtIGu8aWWgz2gkLdI73PGsGO1EhBxADdWMM+sbkRkCQ7z h0VBvjgJqWTEwXslZIZ4IbkjjZvFB44= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=1J6uthgr; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf24.hostedemail.com: domain of jeffxu@google.com designates 209.85.208.51 as permitted sender) smtp.mailfrom=jeffxu@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1716431596; a=rsa-sha256; cv=none; b=zHWkZp7OXPniOHYmeoCO3hzvz5euahyCJbfREt49263Sgq07+bXPA6nm0cZ3/IV851NjC+ 4BG10rS3Ae34BqA4jEPdAEaN+aIUN3+F1A5j4dtfAqcChULtbiv74NN5cCkWvOvToeTGQf njBsZcZXl1epxd8izu4dZyys87iSX0M= Received: by mail-ed1-f51.google.com with SMTP id 4fb4d7f45d1cf-572a1b3d6baso4601a12.1 for ; Wed, 22 May 2024 19:33:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1716431594; x=1717036394; 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=TRtZAXBVtEg5v5aAZ0fCa+gcBJjQ9sT10KYuWxMy/d8=; b=1J6uthgrcwLkarX6RSXssJZ770irrmAHiE0wCH3yXk4dhOMO0SDD4AIwlPg99LlNjl zcItY1dcLPpMsk6VzWiSu+EaEe0Lg107/MWWzbHK6MfZL3x2eFAm2CDyCDEx5kqU/u1/ ttsdJi78uCnRyA43/iyKLmbHLymupbTcLTOhCC+4BeZHaWxz/vqishrVOsohwKOfmNFg ZnO1cbSWAVrctjphVJY830sW4DeFUqHMMFTuCJLWiwNuuTFShdgYpnVdqv/Oa011pRYQ EvbJjamO9TJCOguRJHk5nw53jgQyQo4R0g78dg5GgoXbrRsaQg57yTkXV+u+G2qm3H8R 5x/g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1716431594; x=1717036394; 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=TRtZAXBVtEg5v5aAZ0fCa+gcBJjQ9sT10KYuWxMy/d8=; b=BZ/XkXXxmqWSfJqtfljjEh2oowjo9ffqvdBKSl099V7UG8InO6hFhLfPYahfcfSHOs FXkm1U1YJQdfwpk4bocFXu34MAqf+7vODuUzGGpMkFwcpp/jGjeMaD/NEZ+YfYBAI6Tu 330sVdL+Oe/cfG9Hgg+ulBsEJl42WT6uC+V4+BIAsWxIO4Vk7/xDMMutPtZIAzmEIGep anjxfNgJfPuT0OrF9uQirB618cGuyW9JaliwNYmcbxwUO9OO1eBhwfboVLD+DFpKX1Q3 utKYK+Gpb7c2AJ7ng4k+h2cEF3B9D25qo/4GA3Fcj5d3OOabdhCQGnkILHj8LMAYKowO FYPg== X-Forwarded-Encrypted: i=1; AJvYcCUajwbHLG3fsVNM+H9j/ZGh0rzjbsyNi2E7Xmh7EC0Cvlb/K95bdrFmjW/nWQXOTE4aVfSv0xjbGNZVP1/QU6nolTY= X-Gm-Message-State: AOJu0YwAMuR6MQ8s4p+AQvCpDI7EX7MJBLrjZZeUDH/n1RZt3eGn5PDJ LI2uOWZZG5SqPm3EfvPYhGmVwAMWfRjGaeWRq3BJ81+1GVfRUqBlBieYkfbzaocAgxNbDg+ETS3 NGcbTClgz4dKOZX9c9/mcDxhyoshtaROQZ5fe X-Google-Smtp-Source: AGHT+IFP2LXTGYjyoELOx8sjoR8gHZ7ritbYrH6yk10eTrPzyQaTdWK1XdlYfkD/76p4KOa4GdSBq578XOid7R6AGCM= X-Received: by 2002:a05:6402:3549:b0:572:a154:7081 with SMTP id 4fb4d7f45d1cf-57843f25a14mr100130a12.4.1716431594087; Wed, 22 May 2024 19:33:14 -0700 (PDT) MIME-Version: 1.0 References: <20240513191544.94754-1-pobrn@protonmail.com> <20240522162324.0aeba086228eddd8aff4f628@linux-foundation.org> In-Reply-To: <20240522162324.0aeba086228eddd8aff4f628@linux-foundation.org> From: Jeff Xu Date: Wed, 22 May 2024 19:32:35 -0700 Message-ID: Subject: Re: [PATCH v1] memfd: `MFD_NOEXEC_SEAL` should not imply `MFD_ALLOW_SEALING` To: Andrew Morton Cc: =?UTF-8?B?QmFybmFiw6FzIFDFkWN6ZQ==?= , linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, dmitry.torokhov@gmail.com, dverkamp@chromium.org, hughd@google.com, jorgelo@chromium.org, skhan@linuxfoundation.org, keescook@chromium.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: D3C3218000F X-Stat-Signature: ruhboef19o9cq7zmxx7xebstrqficthb X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1716431595-699447 X-HE-Meta: U2FsdGVkX18DZGJ+pXMSthfwcugKnC610UxAcAXCm9jT/h4fywBMy0IN/rDRlJ2xmXIKUc4V9J1cMJr8fQL8RhPEI/ofi0xezdY++tnYMDBB2uxpPWSPvZtYBIjiJzFcPwFzNscurkD1d+Zce97MQC8pAirgvYRtszbDUig76RGJWvdpyZBGGaViZtQKX/PcFOFMAS7Xmvvl+IEHwT9yx0jXL8C4GtAt/+RTUM11mYHAIxbkIKEW3Msk9jEsnj6FLNMpgSH7TeQqBvbAdvay9UOjn+qz0770+TWSLwoURftNi+emoIbc9fNDDoKvyUt48kPy86M8p/1SmJ25e0EwM6vjYhtF40pc8rLlxRfnd0yXnSZ5aTZiz4HRCHFoOO1iJdhWmSetuJR7+QhcHAsgX6arbFZGacmem9RtLfPyWzq1PjHaBw+QX5QVK8nCFiVGyHKNmeCsD6CqJmSHwOgKGLoLozyUmGR6k5tYLPHqrv/B4dGCH2dkd0goelxlrKxzhPA1oHOe1Y00D4nxETvk8pzwUWPDNyZArZ66xiqjxHrIolZGdA7Y4fRYA3hIyXQx01qyAFkGum3JwKyn0zIKavKtmSB5bcqVDq6qtIE2Tpghcr7GpwH8+VPuGn62ghF0svfgZv4PEBtAPDtW6oLXYu221iLO+/TYvGc6zyWA4ML3o1bwYBZLxFnJU8/m6E+ldmCJey1HjK2r+HnM8WeWo4WGDNO0/nsIQ0f1uZgfFwxEa5P3W0DAYFGtFA5gXlIhyLuhQWEQBRypDHzIRBZnY8bvYpfK+eyoQuKHMnhjCeWD+xS/p5ldbb6MFFrZ2ygN4wAj7M8sWjUMTJyZznXR6ARnxZ6gwx+X9Af5Q9xFLWxcYaaKTPW20iXIy0bV2ma5tWhDzASyBMyIcW9JEc7klf/v9KoXjjefF8khUjtljVhhV/RRI188vTkDYX+BvA6uq9OjSd2ErmuU778EhJG B7D8dcuy Wk6c1NSvinuAxlgbpwsdfcwcWrZ0dZSR2U+Om13CJgZpzbei0BgohnqozIs7MT+aEUEi5chZi1P0GrXGMJeobIoWvz5FFWi0M1sIhG/GNxmNEQcnxpVZe5kJBvhYH4dHMOqn9NrDI4tqCmN7hXxSkwwZJoUno3vLiyyoaWTCAQddW+lhdvMKlA2/xFINUQArXYU5ksNSGskzGb/I4CcAIIN0OcGG5sQXZYnL7qw/RGdXy+jQVCgTthe3DN3sW642Amc6aCxqYNPk+lSILNeFqCiKuyH2t49PJXoZfWFIr0oIAfbES7bACD5AiD+SO6RyFgl1VniyKEMjBTmhB2N/HKhagBGPYSDBN/y6OHK3LsEninrqFHXuYe4ZAXIciANQtNRuOEcuurPA/cJ477c1MqmsV8EJQHWeyl7e/PSVM5/RaOnGIURUCA83pXRjxJKJNHJlz6G4haJO4XjfLwdEwyb6t/KAaxRCTj9RtaqVFN1NER86zNL1v5sCMPgjvBMYt8wqT5P0zZHmeBYVmCuecTIFVn0SIonyfYOib X-Bogosity: Ham, tests=bogofilter, spamicity=0.000005, 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 Wed, May 22, 2024 at 4:23=E2=80=AFPM Andrew Morton wrote: > > On Wed, 15 May 2024 23:11:12 -0700 Jeff Xu wrote: > > > On Mon, May 13, 2024 at 12:15=E2=80=AFPM Barnab=C3=A1s P=C5=91cze wrote: > > > > > > `MFD_NOEXEC_SEAL` should remove the executable bits and set > > > `F_SEAL_EXEC` to prevent further modifications to the executable > > > bits as per the comment in the uapi header file: > > > > > > not executable and sealed to prevent changing to executable > > > > > > However, currently, it also unsets `F_SEAL_SEAL`, essentially > > > acting as a superset of `MFD_ALLOW_SEALING`. Nothing implies > > > that it should be so, and indeed up until the second version > > > of the of the patchset[0] that introduced `MFD_EXEC` and > > > `MFD_NOEXEC_SEAL`, `F_SEAL_SEAL` was not removed, however it > > > was changed in the third revision of the patchset[1] without > > > a clear explanation. > > > > > > This behaviour is suprising for application developers, > > > there is no documentation that would reveal that `MFD_NOEXEC_SEAL` > > > has the additional effect of `MFD_ALLOW_SEALING`. > > > > > Ya, I agree that there should be documentation, such as a man page. I w= ill > > work on that. > > > > > So do not remove `F_SEAL_SEAL` when `MFD_NOEXEC_SEAL` is requested. > > > This is technically an ABI break, but it seems very unlikely that an > > > application would depend on this behaviour (unless by accident). > > > > > > [0]: https://lore.kernel.org/lkml/20220805222126.142525-3-jeffxu@goog= le.com/ > > > [1]: https://lore.kernel.org/lkml/20221202013404.163143-3-jeffxu@goog= le.com/ > > > > ... > > > > Reviewed-by: Jeff Xu > > It's a change to a userspace API, yes? Please let's have a detailed > description of why this is OK. Why it won't affect any existing users. > Unfortunately, this is a breaking change that might break a application if they do below: memfd_create("", MFD_NOEXEC_SEAL) fcntl(fd, F_ADD_SEALS, F_SEAL_WRITE); <-- this will fail in new semantics, due to mfd not being sealable. However, I still think the new semantics is a better, the reason is due to the sysctl: memfd_noexec_scope Currently, when the sysctl is set to MEMFD_NOEXEC_SCOPE_NOEXEC_SEAL kernel adds MFD_NOEXEC_SEAL to memfd_create, and the memfd becomes sealabl= e. E.g. When the sysctl is set to MEMFD_NOEXEC_SCOPE_NOEXEC_SEAL The app calls memfd_create("",0) application will get sealable memfd, which might be a surprise to applicati= on. If the app doesn't want this behavior, they will need one of two below in current implementation. 1> set the sysctl: memfd_noexec_scope to 0. So the kernel doesn't overwrite the mdmfd_create 2> modify their code to get non-sealable NOEXEC memfd. memfd_create("", MEMFD_NOEXEC_SCOPE_NOEXEC) fcntl(fd, F_ADD_SEALS, F_SEAL_SEAL) The new semantics works better with the sysctl. Since memfd noexec is new, maybe there is no application using the MFD_NOEXEC_SEAL to create sealable memfd. They mostly likely use memfd(MFD_NOEXEC_SEAL|MFD_ALLOW_SEALING) instead. I think it might benefit in the long term with the new semantics. If breaking change is not recommended, the alternative is to introduce a new flag. MFD_NOEXEC_SEAL_SEAL. (I can't find a better name...) What do you think ? > Also, please let's give consideration to a -stable backport so that all > kernel versions will eventually behave in the same manner. > Yes. If the new semantics is acceptable, backport is needed as bugfix to all kernel versions. I can do that if someone helps me with the process. And sorry about this bug that I created.