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 E8C3AC4332F for ; Thu, 8 Dec 2022 16:27:07 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5054A8E0003; Thu, 8 Dec 2022 11:27:07 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 48E9E8E0001; Thu, 8 Dec 2022 11:27:07 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3088F8E0003; Thu, 8 Dec 2022 11:27:07 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 1D3928E0001 for ; Thu, 8 Dec 2022 11:27:07 -0500 (EST) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id B7C54AAFB2 for ; Thu, 8 Dec 2022 16:27:06 +0000 (UTC) X-FDA: 80219668452.26.BBE5884 Received: from mail-pf1-f170.google.com (mail-pf1-f170.google.com [209.85.210.170]) by imf07.hostedemail.com (Postfix) with ESMTP id 6BFEF40011 for ; Thu, 8 Dec 2022 16:27:03 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=DIt8YjYL; spf=pass (imf07.hostedemail.com: domain of keescook@chromium.org designates 209.85.210.170 as permitted sender) smtp.mailfrom=keescook@chromium.org; dmarc=pass (policy=none) header.from=chromium.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1670516823; a=rsa-sha256; cv=none; b=H9bliznAgKi42pFAhu8MJCebXGMMBeJznrEVSjBwIlvLRSK98MAmOzlLLOpqqHqzwMstTt hsHhZWLgrtMgX8fC7UhCV91sHUa4HiqvF3QW45kvWTZfaLtQnVnNzH4/5dBAeWkf4nwpTZ a3+ZsP2SjpWeMmk5czq8slEcXVX1rfM= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=DIt8YjYL; spf=pass (imf07.hostedemail.com: domain of keescook@chromium.org designates 209.85.210.170 as permitted sender) smtp.mailfrom=keescook@chromium.org; dmarc=pass (policy=none) header.from=chromium.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1670516823; 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=7Lh+BTg81soVr+6LzeJ2EjyFbH2jfE8Jb6jdCOQpIEs=; b=MvVoXyXENxeUoOZKEIy3ZL31pbDlxSP4XG58M/xmtoThqmGWYkcOS1c6YPOBboA4eczI4v q4xUB0KQwTXW4JfTf6S0eWVjT7Hq9aPImlh+FMvdBZqW1SPIzuMVXO6a2JAqQgdu5jwUeC h/mraVbugPUqPGoTPCe+CFwYWtXoKCI= Received: by mail-pf1-f170.google.com with SMTP id t18so1671721pfq.13 for ; Thu, 08 Dec 2022 08:27:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; 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=7Lh+BTg81soVr+6LzeJ2EjyFbH2jfE8Jb6jdCOQpIEs=; b=DIt8YjYLc0xFCDPfw1Lw7trfvvmJVfLf3tbVp+MOv4p+gDV3qSW8hpIllyH3D/iAVb bKq4FcwS5F96gVeKpHTemDe51zgq7iQbixfvk819Rpq6ehz01tUmK0FERmnoStBi2WMX 6XtN+1lurNUxjL/FWvh3/eNatm0QemovgNND4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=7Lh+BTg81soVr+6LzeJ2EjyFbH2jfE8Jb6jdCOQpIEs=; b=Fu4kR3E24fFwe2Nfnj7vf8nmqgM5kJZ1kG2DVtq6iPzKsYXUvo8zSjiG03FFtpwJyO mVR/YHcfjgrbA0Zo2aJByOENB5mDXcvPa/rw7GS52B9msjUhwYb2FVdHrMJXWEblCk7Z sAlMcSieef8nw8rNW8j4U9r0o1k8ASe81ACIoPyfACgUjgXoBhGE2E2RotCThVBt+sqx obE3y0n72XwV50eywv2ctz3SlNVdZKZFc+hYzu8F0u9f4nUnONMSfgQ6iVfA6tBWQswV ZB/HPxKP4EVDPH8gRfQ/SVwWHoxmzEGknE8v5mRyNo9ITm7akZdEZZY2GLPOR0d6hqiD e1TA== X-Gm-Message-State: ANoB5pn9DQUNDj38UTapLfJCEdMDAAt0ydwnNvM/EpH6Bz/mUeJJS9Qb Tt1nDa3qba0l0R2i/+U6VRwsJA== X-Google-Smtp-Source: AA0mqf6Xr6hLzp/usF6hIGA8rv4AdkSlLfhKsnnSzQoxC/SeedquZho16Iu7SkOY+sazrrQNl421pA== X-Received: by 2002:a05:6a02:108:b0:46b:3acb:77ab with SMTP id bg8-20020a056a02010800b0046b3acb77abmr67350434pgb.359.1670516822007; Thu, 08 Dec 2022 08:27:02 -0800 (PST) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id u68-20020a626047000000b0057716769289sm6867426pfb.196.2022.12.08.08.27.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 08 Dec 2022 08:27:01 -0800 (PST) Date: Thu, 8 Dec 2022 08:27:00 -0800 From: Kees Cook To: jeffxu@chromium.org Cc: skhan@linuxfoundation.org, akpm@linux-foundation.org, dmitry.torokhov@gmail.com, dverkamp@chromium.org, hughd@google.com, jeffxu@google.com, jorgelo@chromium.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-mm@kvack.org, jannh@google.com, linux-hardening@vger.kernel.org, kernel test robot Subject: Re: [PATCH v6 3/6] mm/memfd: add MFD_NOEXEC_SEAL and MFD_EXEC Message-ID: <202212080821.5AE7EE99@keescook> References: <20221207154939.2532830-1-jeffxu@google.com> <20221207154939.2532830-4-jeffxu@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20221207154939.2532830-4-jeffxu@google.com> X-Rspam-User: X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 6BFEF40011 X-Stat-Signature: 3bk344qb578r64iozc6fz117yydye4jm X-HE-Tag: 1670516823-913756 X-HE-Meta: U2FsdGVkX1+Fg95m61fOAuIJb7ghHciptYWYtRUChEvU1xvn8Hy9pumlv6Gu5bWeyW5MYwMDrR2Vt50xUTgaeiY3bYJJi51Ei+OW8uFvAs56MsM0OfjwFmiMIU98fQtRvdzwzFkG7GiJRlPJjJufXYvLoTlSdUYddBPYcT5lBTL6dy7EwqD9/XM7LJW99ufX2GLeSZ6JewhZYMLDe6niaDFLfCf4vmBDLktWfZPTf8wSu5gESAY7hUqdjAKgVZMMfw6kKeXY63+NLpqciIdV75goKPjxmtoHD4g24xhnWTXrHPeKsM0ghzj1miXy1vjjYiak4RMEboXSdQHusdwqLlbeytZUr5KW/fYn92cmNL8cEzrxoEEK97PDwIA/QhDFIygUvSWzBJXMO2iBJXYoU8+bJ0d0amtHf5Kyh08C3zvAXIFXKjaInD7qIFNRJFfcqq9Was8VmaJ3bXrG20itDowSbMg9MpgWQzv63uLj/GuNKrPl3Q2PxO2c8GtwRNpPW1Jx885AOivpQgFjPo3VX6Xlr16BRwmjfEwUPJFFBqlCIY9+2Q/Q1zfdiaVCzocBWVSQRsQSTLQYiZYXvzYHTb076Z1cd+zYNBgIPwEzNfIHIMDluWxZxT5f9FmCID0bjPDyoM08WHJGINkEZDZtgkU6IObSSfmRpbk07d2XTDtcA/hxRbJF/gVNaZBGDzysjZo0hYFwrprvm//g9X25wO5uV0QHXHYnkScWmrzQiqzHj1+qZMMoNOhMVPan4i39RPYwKKhotkSwSHI8EeyuGt7afWU2dPuG59P1hdOibBTFbbakHODR2wTzJD/8n4BwPnFwJi7VA45i7QCKilonBc7Q+ONXzR1wKh0wkAoXCX6T5ul9ROXCG6trWtyjel2/IUjPvxa+oMTVIzv5nfl+GNAoJuLV6rVbyHa4NDdEH2oajg06fvKAg1ALK6uE1AmizzNNoVujZSNDdrbUbYj eHKiSFX6 GT+XFhXJOlE22QdAI/rSvIRn3UfETxeTGkbeZQOA0fRW7XYVkMi+Y2B5IHaigwUFcDHgOhzEGRHhtctc52yRMD4JpZChY1EobCCWhWEH21Ofi99C8epEhvflZxUqCK+L2S3Gg2RTcV9KTfYY/cEKdufeMTzjqFOkBood4Lh/P+NE1R/kZ2YUfxOz2XT9glDoQQy0FMiSsdmRdL0h/jzeqsGrwaw== 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: On Wed, Dec 07, 2022 at 03:49:36PM +0000, jeffxu@chromium.org wrote: > From: Jeff Xu > > The new MFD_NOEXEC_SEAL and MFD_EXEC flags allows application to > set executable bit at creation time (memfd_create). > > When MFD_NOEXEC_SEAL is set, memfd is created without executable bit > (mode:0666), and sealed with F_SEAL_EXEC, so it can't be chmod to > be executable (mode: 0777) after creation. > > when MFD_EXEC flag is set, memfd is created with executable bit > (mode:0777), this is the same as the old behavior of memfd_create. > > The new pid namespaced sysctl vm.memfd_noexec has 3 values: > 0: memfd_create() without MFD_EXEC nor MFD_NOEXEC_SEAL acts like > MFD_EXEC was set. > 1: memfd_create() without MFD_EXEC nor MFD_NOEXEC_SEAL acts like > MFD_NOEXEC_SEAL was set. > 2: memfd_create() without MFD_NOEXEC_SEAL will be rejected. > > The sysctl allows finer control of memfd_create for old-software > that doesn't set the executable bit, for example, a container with > vm.memfd_noexec=1 means the old-software will create non-executable > memfd by default. Also, the value of memfd_noexec is passed to child > namespace at creation time. For example, if the init namespace has > vm.memfd_noexec=2, all its children namespaces will be created with 2. > > Signed-off-by: Jeff Xu > Co-developed-by: Daniel Verkamp > Signed-off-by: Daniel Verkamp > Reported-by: kernel test robot Please rearrange these tags, and add a link to the lkp report: Reported-by: kernel test robot Link: ...url.to.lkp.lore.email... Co-developed-by: Daniel Verkamp Signed-off-by: Daniel Verkamp Signed-off-by: Jeff Xu > --- > include/linux/pid_namespace.h | 19 +++++++++++ > include/uapi/linux/memfd.h | 4 +++ > kernel/pid_namespace.c | 5 +++ > kernel/pid_sysctl.h | 59 +++++++++++++++++++++++++++++++++++ > mm/memfd.c | 48 ++++++++++++++++++++++++++-- > 5 files changed, 133 insertions(+), 2 deletions(-) > create mode 100644 kernel/pid_sysctl.h > > diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h > index 07481bb87d4e..a4789a7b34a9 100644 > --- a/include/linux/pid_namespace.h > +++ b/include/linux/pid_namespace.h > @@ -16,6 +16,21 @@ > > struct fs_pin; > > +#if defined(CONFIG_SYSCTL) && defined(CONFIG_MEMFD_CREATE) > +/* > + * sysctl for vm.memfd_noexec > + * 0: memfd_create() without MFD_EXEC nor MFD_NOEXEC_SEAL > + * acts like MFD_EXEC was set. > + * 1: memfd_create() without MFD_EXEC nor MFD_NOEXEC_SEAL > + * acts like MFD_NOEXEC_SEAL was set. > + * 2: memfd_create() without MFD_NOEXEC_SEAL will be > + * rejected. > + */ > +#define MEMFD_NOEXEC_SCOPE_EXEC 0 > +#define MEMFD_NOEXEC_SCOPE_NOEXEC_SEAL 1 > +#define MEMFD_NOEXEC_SCOPE_NOEXEC_ENFORCED 2 These don't align? I think a tab is missing on MEMFD_NOEXEC_SCOPE_EXEC. > +#endif > + > struct pid_namespace { > struct idr idr; > struct rcu_head rcu; > @@ -31,6 +46,10 @@ struct pid_namespace { > struct ucounts *ucounts; > int reboot; /* group exit code if this pidns was rebooted */ > struct ns_common ns; > +#if defined(CONFIG_SYSCTL) && defined(CONFIG_MEMFD_CREATE) > + /* sysctl for vm.memfd_noexec */ > + int memfd_noexec_scope; > +#endif > } __randomize_layout; > > extern struct pid_namespace init_pid_ns; > diff --git a/include/uapi/linux/memfd.h b/include/uapi/linux/memfd.h > index 7a8a26751c23..273a4e15dfcf 100644 > --- a/include/uapi/linux/memfd.h > +++ b/include/uapi/linux/memfd.h > @@ -8,6 +8,10 @@ > #define MFD_CLOEXEC 0x0001U > #define MFD_ALLOW_SEALING 0x0002U > #define MFD_HUGETLB 0x0004U > +/* not executable and sealed to prevent changing to executable. */ > +#define MFD_NOEXEC_SEAL 0x0008U > +/* executable */ > +#define MFD_EXEC 0x0010U > > /* > * Huge page size encoding when MFD_HUGETLB is specified, and a huge page > diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c > index f4f8cb0435b4..8a98b1af9376 100644 > --- a/kernel/pid_namespace.c > +++ b/kernel/pid_namespace.c > @@ -23,6 +23,7 @@ > #include > #include > #include > +#include "pid_sysctl.h" > > static DEFINE_MUTEX(pid_caches_mutex); > static struct kmem_cache *pid_ns_cachep; > @@ -110,6 +111,8 @@ static struct pid_namespace *create_pid_namespace(struct user_namespace *user_ns > ns->ucounts = ucounts; > ns->pid_allocated = PIDNS_ADDING; > > + initialize_memfd_noexec_scope(ns); > + > return ns; > > out_free_idr: > @@ -455,6 +458,8 @@ static __init int pid_namespaces_init(void) > #ifdef CONFIG_CHECKPOINT_RESTORE > register_sysctl_paths(kern_path, pid_ns_ctl_table); > #endif > + > + register_pid_ns_sysctl_table_vm(); > return 0; > } > > diff --git a/kernel/pid_sysctl.h b/kernel/pid_sysctl.h > new file mode 100644 > index 000000000000..5986d6493b5b > --- /dev/null > +++ b/kernel/pid_sysctl.h > @@ -0,0 +1,59 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef LINUX_PID_SYSCTL_H > +#define LINUX_PID_SYSCTL_H > + > +#include > + > +#if defined(CONFIG_SYSCTL) && defined(CONFIG_MEMFD_CREATE) > +static inline void initialize_memfd_noexec_scope(struct pid_namespace *ns) > +{ > + ns->memfd_noexec_scope = > + task_active_pid_ns(current)->memfd_noexec_scope; > +} > + > +static int pid_mfd_noexec_dointvec_minmax(struct ctl_table *table, > + int write, void *buf, size_t *lenp, loff_t *ppos) > +{ > + struct pid_namespace *ns = task_active_pid_ns(current); > + struct ctl_table table_copy; > + > + if (write && !capable(CAP_SYS_ADMIN)) > + return -EPERM; Should this be CAP_SYS_ADMIN within the userns, rather than the global init_task CAP_SYS_ADMIN? > + > + table_copy = *table; > + if (ns != &init_pid_ns) > + table_copy.data = &ns->memfd_noexec_scope; > + > + /* > + * set minimum to current value, the effect is only bigger > + * value is accepted. > + */ > + if (*(int *)table_copy.data > *(int *)table_copy.extra1) > + table_copy.extra1 = table_copy.data; > + > + return proc_dointvec_minmax(&table_copy, write, buf, lenp, ppos); > +} > + > +static struct ctl_table pid_ns_ctl_table_vm[] = { > + { > + .procname = "memfd_noexec", > + .data = &init_pid_ns.memfd_noexec_scope, > + .maxlen = sizeof(init_pid_ns.memfd_noexec_scope), > + .mode = 0644, > + .proc_handler = pid_mfd_noexec_dointvec_minmax, > + .extra1 = SYSCTL_ZERO, > + .extra2 = SYSCTL_TWO, > + }, > + { } > +}; > +static struct ctl_path vm_path[] = { { .procname = "vm", }, { } }; > +static inline void register_pid_ns_sysctl_table_vm(void) > +{ > + register_sysctl_paths(vm_path, pid_ns_ctl_table_vm); > +} > +#else > +static inline void set_memfd_noexec_scope(struct pid_namespace *ns) {} > +static inline void register_pid_ns_ctl_table_vm(void) {} > +#endif > + > +#endif /* LINUX_PID_SYSCTL_H */ > diff --git a/mm/memfd.c b/mm/memfd.c > index 4ebeab94aa74..ec70675a7069 100644 > --- a/mm/memfd.c > +++ b/mm/memfd.c > @@ -18,6 +18,7 @@ > #include > #include > #include > +#include > #include > > /* > @@ -263,12 +264,14 @@ long memfd_fcntl(struct file *file, unsigned int cmd, unsigned long arg) > #define MFD_NAME_PREFIX_LEN (sizeof(MFD_NAME_PREFIX) - 1) > #define MFD_NAME_MAX_LEN (NAME_MAX - MFD_NAME_PREFIX_LEN) > > -#define MFD_ALL_FLAGS (MFD_CLOEXEC | MFD_ALLOW_SEALING | MFD_HUGETLB) > +#define MFD_ALL_FLAGS (MFD_CLOEXEC | MFD_ALLOW_SEALING | MFD_HUGETLB | MFD_NOEXEC_SEAL | MFD_EXEC) > > SYSCALL_DEFINE2(memfd_create, > const char __user *, uname, > unsigned int, flags) > { > + char comm[TASK_COMM_LEN]; I'm fine with using "comm", but technically, it's not needed: task->comm will always be %NUL terminated. > + struct pid_namespace *ns; > unsigned int *file_seals; > struct file *file; > int fd, error; > @@ -285,6 +288,39 @@ SYSCALL_DEFINE2(memfd_create, > return -EINVAL; > } > > + /* Invalid if both EXEC and NOEXEC_SEAL are set.*/ > + if ((flags & MFD_EXEC) && (flags & MFD_NOEXEC_SEAL)) > + return -EINVAL; > + > + if (!(flags & (MFD_EXEC | MFD_NOEXEC_SEAL))) { > +#ifdef CONFIG_SYSCTL > + int sysctl = MEMFD_NOEXEC_SCOPE_EXEC; > + > + ns = task_active_pid_ns(current); > + if (ns) > + sysctl = ns->memfd_noexec_scope; > + > + switch (sysctl) { > + case MEMFD_NOEXEC_SCOPE_EXEC: > + flags |= MFD_EXEC; > + break; > + case MEMFD_NOEXEC_SCOPE_NOEXEC_SEAL: > + flags |= MFD_NOEXEC_SEAL; > + break; > + default: > + pr_warn_ratelimited( > + "memfd_create(): MFD_NOEXEC_SEAL is enforced, pid=%d '%s'\n", > + task_pid_nr(current), get_task_comm(comm, current)); > + return -EINVAL; > + } > +#else > + flags |= MFD_EXEC; > +#endif > + pr_warn_ratelimited( > + "memfd_create() without MFD_EXEC nor MFD_NOEXEC_SEAL, pid=%d '%s'\n", > + task_pid_nr(current), get_task_comm(comm, current)); > + } > + > /* length includes terminating zero */ > len = strnlen_user(uname, MFD_NAME_MAX_LEN + 1); > if (len <= 0) > @@ -328,7 +364,15 @@ SYSCALL_DEFINE2(memfd_create, > file->f_mode |= FMODE_LSEEK | FMODE_PREAD | FMODE_PWRITE; > file->f_flags |= O_LARGEFILE; > > - if (flags & MFD_ALLOW_SEALING) { > + if (flags & MFD_NOEXEC_SEAL) { > + struct inode *inode = file_inode(file); > + > + inode->i_mode &= ~0111; > + file_seals = memfd_file_seals_ptr(file); > + *file_seals &= ~F_SEAL_SEAL; > + *file_seals |= F_SEAL_EXEC; > + } else if (flags & MFD_ALLOW_SEALING) { > + /* MFD_EXEC and MFD_ALLOW_SEALING are set */ > file_seals = memfd_file_seals_ptr(file); > *file_seals &= ~F_SEAL_SEAL; > } > -- > 2.39.0.rc0.267.gcb52ba06e7-goog > Otherwise looks good! -- Kees Cook