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 6099BE77188 for ; Mon, 6 Jan 2025 17:35:25 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DA7AE6B0082; Mon, 6 Jan 2025 12:35:24 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id D575B6B0083; Mon, 6 Jan 2025 12:35:24 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C45E96B0089; Mon, 6 Jan 2025 12:35:24 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id A775B6B0082 for ; Mon, 6 Jan 2025 12:35:24 -0500 (EST) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 615B116018F for ; Mon, 6 Jan 2025 17:35:24 +0000 (UTC) X-FDA: 82977728568.03.C18D730 Received: from mail-oa1-f48.google.com (mail-oa1-f48.google.com [209.85.160.48]) by imf26.hostedemail.com (Postfix) with ESMTP id 65839140016 for ; Mon, 6 Jan 2025 17:35:22 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=X6g0sIXX; spf=pass (imf26.hostedemail.com: domain of jeffxu@chromium.org designates 209.85.160.48 as permitted sender) smtp.mailfrom=jeffxu@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=1736184922; 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=pKp7aPmOTHvMER9QV0OoIt9TQh4BPYke1ArM9VzzgAw=; b=8BUWQXiqfkZN+JJFbUsStjXjJP8mqwIPqWiFtYADZTYTapndK3UKEp1xs/pEnYweLZWFPA JnV2VqOCDTitlWYQEcgJ7yPmI3Kydy/Amr+8scHPKrH4xBGa3+pmbvZU4Ye3vy5wRXX3Uk c+XVyupB9HJhz/wn/Rgj6JT2Dv9HXJg= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1736184922; a=rsa-sha256; cv=none; b=gHkFJSCZrTDd/VE1GEv9XEm9TCCtwIXvjyW4FQP+8j1hOIu8YLb9zK4kpmNh7LKr6St2fI NASiHMsZme3spyonoeJJpHNqnkSRLMj9/0FciMudBrbE7mp5e1J/IA3k2q/iLkzWhZ6UrF 7VjO00hbueYgEvmkG7yh7IbX5CudYDA= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=X6g0sIXX; spf=pass (imf26.hostedemail.com: domain of jeffxu@chromium.org designates 209.85.160.48 as permitted sender) smtp.mailfrom=jeffxu@chromium.org; dmarc=pass (policy=none) header.from=chromium.org Received: by mail-oa1-f48.google.com with SMTP id 586e51a60fabf-268d0979e90so1408749fac.3 for ; Mon, 06 Jan 2025 09:35:22 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1736184921; x=1736789721; 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=pKp7aPmOTHvMER9QV0OoIt9TQh4BPYke1ArM9VzzgAw=; b=X6g0sIXXMw9Zc24/Esys6HCOeM0caZY2LopGRJ/xl4lnb+mq9C1Vh5v2KiDhyWyEpr SNR39ynzr2j9oomvQNFXoveIPRHjyPbw3r4r8zfIADFVCCCGOy+Oqoyx/urx1jWqk66F gB0eGF6i+IRFmi/V0PCQndwzsI1yf1/Yw97eo= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736184921; x=1736789721; 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=pKp7aPmOTHvMER9QV0OoIt9TQh4BPYke1ArM9VzzgAw=; b=rog6TKVlR/GrXYtOCUfVrYPjqjyf065oeaKWM27sa90fxegkIOLQU9Q8TAomSr44Ub b/tMvLoN1qdcAsG9mrUbzXxlIJEuKJmjwv7cKvn0yRlz21Q9qQvsLt7Ln8dlbV0BcmHx I+u5E6Dis7vnWmbzeX3vsZdi3HLh0sAl/X9F96YaRNPab0sFboweeh0koTXS7C5b+XTC lvBOFmQXPbjUnQJ0G0tSoR5FgreR/AdwI9+MCWmKfyT7NSSX8IZ+fAMu/c8c5a746Zcv L2CoBzqF23pOMeauBx4+V0qisxPmhyqodFAsWgvxSQ33JHHXusHRsk9v0TALOQZOFN/U YHew== X-Forwarded-Encrypted: i=1; AJvYcCW/SUQZcbqDg97BsjejHCs72YRIMwlJxNDgXhvZUGSxFY43rUFmuM9Ih+cwMGBRzTJAI/qGS+1X+Q==@kvack.org X-Gm-Message-State: AOJu0YxMjU0rgalhhDazgO8Wp04ldqpOHBv5nYEvluQYRxfbl4TW2gol 4rgfsSh6Gnse0mpgXbjun39pV0wO4QmX2hkiBu247Ed7iBviL9pGmvMkyvUJUEpG3Me0jjYtQFA vS6Xki9l2SzDKxBLfiERi3UmrO7jfVW/s+KVW X-Gm-Gg: ASbGncvwXnQ4eWUSkanpV/dggfNO8adWeX8EVwAXDjBuAEWJCjnFEcdXQeZ9YG9SARq waU1rEmG6+TBXdZzLsdICzIZ5PuvpMxqUN+sabywb+ZLKut31fKIen0EVGBZdOG//sls= X-Google-Smtp-Source: AGHT+IFKUMHqJMC7OwYbzyAXF8vzNPgoUNTgojG07VvQbtPqbUNXWJfoqydAkiw3Lx9C8McHpEeXmpt31qs2elbcKE0= X-Received: by 2002:a05:6870:9a21:b0:29e:49f7:f456 with SMTP id 586e51a60fabf-2a7fb16cf2amr10241213fac.7.1736184921130; Mon, 06 Jan 2025 09:35:21 -0800 (PST) MIME-Version: 1.0 References: <20250102233255.1180524-1-isaacmanjarres@google.com> <20250102233255.1180524-2-isaacmanjarres@google.com> In-Reply-To: From: Jeff Xu Date: Mon, 6 Jan 2025 09:35:09 -0800 Message-ID: Subject: Re: [RFC PATCH RESEND v2 1/2] mm/memfd: Add support for F_SEAL_FUTURE_EXEC to memfd To: Jann Horn , Kees Cook Cc: "Isaac J. Manjarres" , lorenzo.stoakes@oracle.com, Jeff Layton , Chuck Lever , Alexander Aring , Andrew Morton , Shuah Khan , surenb@google.com, kaleshsingh@google.com, jstultz@google.com, aliceryhl@google.com, jeffxu@google.com, kees@kernel.org, kernel-team@android.com, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-kselftest@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 65839140016 X-Rspam-User: X-Rspamd-Server: rspam07 X-Stat-Signature: i1mt1d4ywp1raot8og6bddq7rgwpu69a X-HE-Tag: 1736184922-709088 X-HE-Meta: U2FsdGVkX1+fyqM50p1rLNJclzq+9EV+hs2b3Y5MJXInYT2qHUi3wIb/yo3eZfHfbD9+T/ML4Sj3wfpd9uQkFj8gZfhyvruKS2MxGzIf33AwxkyTFXj/HCvc82SX3oAAlw/7rIkLUbgSEwmD179ilUYku3bcXZ+xEP4KCYjtiHajZDOtcPfsQSL/hQUqe8nEYTTOb4ZtRXFtE4py+bqje+C83cv5te79JYMovpnjLBPISRyaA2js4of6mzaW1rHxIKigfJWptd2L0zNeHED7KBBqpADEoZgr4LrJcT1/5VAFH7xstgSjKc73W11iOjRi6DjeOMZ+f6WanIeximHdKI3vF9GrIf2tDrnTX4LDd1c+NZ+Mv3hwSJlwbk+SX528a68E6q109pf9VoeeG0yUYZhkj3JKcbbHX510Hnzp25RxhtSUWZAAeW+a7Ob+/CO5mg0fA2NwOqDzRPa2oHSn1UT4PEh8MzvBCxwRAESe8Z2y3esazY4bQxyRc43sbvFH6UH1ofhvizfLGn4sJEhLzNtub37Fk/O0f99HpM7wSLuF3TIcajO3a9sMIWI+Fwzpx14MwDwMyvtl3QJb3Axe/rKj1JQIHLl6Z/NU3ywZikE6LG42hmvTZvXxbgpRFGNdEr1e1k6s5JaOxle72KxLluP8kY1Jx783A5+t7fScA6MleXKwD6TvdEzS37J2ZElTKg3RkguJxgFlYfNqXJ1n+L4jlQoqG7vTfSz2a/qKolUJJqM879Tb+9qPJm9/4W0cRJI4X7jWt0PI62PSFBt+xYxPxKwpe0pJOL3q7GvotmzFuxvgXBhwfjJrmOY+LzZ+iiOb7GxEjdtFtwFiIdj+E2eJQmUQYOHU5TXRgRrsi9/OjB/UFZLZalZlQUkme3X/zZY0YjtoGbxoF+R1UEiBk2q/MB0MtJRmiaZVIXQIk5r1fVE59D4Ygc9ZHmL8QSSyoA3jcv5oZy6g8IRi79d 2Ols+mEq d/ZaGwfbiqthxalmOAt1zfMl+eDL1zGTLf/r0e3Y0IUj185pAMLqteV1rWGqLkNoiu2NIWjqLMx3yILMbZAXdyCab2mSPH7qAqIy2mPbBk6L5AJfitOvtpNoF0dfHllxQQ4jyJLfbxT0IdsnseP3M9r6kHZMVbMd0WljJUGPPor+Whr1MGxQQtjY5HuH6mV8Hm3/Mqamj8etvpj4U4f4159FDfgQWJXUqX8MKIoWN0Inuavn0Kps9S+eIenvA67rZHmHedhvaV+zqpFdYOkxR/HBEC4W4UPECZYEullV56OzMrtHRschKiJQnv+n5whxJCxr7Q5P89Eh7kIki+55zh1pruax3FHCiIiolHSSBvM7lpu0= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000060, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: + Kees because this is related to W^X memfd and security. On Fri, Jan 3, 2025 at 7:04=E2=80=AFAM Jann Horn wrote: > > On Fri, Jan 3, 2025 at 12:32=E2=80=AFAM Isaac J. Manjarres > wrote: > > Android currently uses the ashmem driver [1] for creating shared memory > > regions between processes. Ashmem buffers can initially be mapped with > > PROT_READ, PROT_WRITE, and PROT_EXEC. Processes can then use the > > ASHMEM_SET_PROT_MASK ioctl command to restrict--never add--the > > permissions that the buffer can be mapped with. > > > > Processes can remove the ability to map ashmem buffers as executable to > > ensure that those buffers cannot be exploited to run unintended code. > > Is there really code out there that first maps an ashmem buffer with > PROT_EXEC, then uses the ioctl to remove execute permission for future > mappings? I don't see why anyone would do that. > > > For instance, suppose process A allocates a memfd that is meant to be > > read and written by itself and another process, call it B. > > > > Process A shares the buffer with process B, but process B injects code > > into the buffer, and compromises process A, such that it makes A map > > the buffer with PROT_EXEC. This provides an opportunity for process A > > to run the code that process B injected into the buffer. > > > > If process A had the ability to seal the buffer against future > > executable mappings before sharing the buffer with process B, this > > attack would not be possible. > > I think if you want to enforce such restrictions in a scenario where > the attacker can already make the target process perform > semi-arbitrary syscalls, it would probably be more reliable to enforce > rules on executable mappings with something like SELinux policy and/or > F_SEAL_EXEC. > I would like to second on the suggestion of making this as part of F_SEAL_= EXEC. > > Android is currently trying to replace ashmem with memfd. However, memf= d > > does not have a provision to permanently remove the ability to map a > > buffer as executable, and leaves itself open to the type of attack > > described earlier. However, this should be something that can be > > achieved via a new file seal. > > > > There are known usecases (e.g. CursorWindow [2]) where a process > > maps a buffer with read/write permissions before restricting the buffer > > to being mapped as read-only for future mappings. > > Here you're talking about write permission, but the patch is about > execute permission? > > > The resulting VMA from the writable mapping has VM_MAYEXEC set, meaning > > that mprotect() can change the mapping to be executable. Therefore, > > implementing the seal similar to F_SEAL_WRITE would not be appropriate, > > since it would not work with the CursorWindow usecase. This is because > > the CursorWindow process restricts the mapping permissions to read-only > > after the writable mapping is created. So, adding a file seal for > > executable mappings that operates like F_SEAL_WRITE would fail. > > > > Therefore, add support for F_SEAL_FUTURE_EXEC, which is handled > > similarly to F_SEAL_FUTURE_WRITE. This ensures that CursorWindow can > > continue to create a writable mapping initially, and then restrict the > > permissions on the buffer to be mappable as read-only by using both > > F_SEAL_FUTURE_WRITE and F_SEAL_FUTURE_EXEC. After the seal is > > applied, any calls to mmap() with PROT_EXEC will fail. > > > > [1] https://cs.android.com/android/kernel/superproject/+/common-android= -mainline:common/drivers/staging/android/ashmem.c > > [2] https://developer.android.com/reference/android/database/CursorWind= ow > > > > Signed-off-by: Isaac J. Manjarres > > --- > > include/uapi/linux/fcntl.h | 1 + > > mm/memfd.c | 39 +++++++++++++++++++++++++++++++++++++- > > 2 files changed, 39 insertions(+), 1 deletion(-) > > > > diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h > > index 6e6907e63bfc..ef066e524777 100644 > > --- a/include/uapi/linux/fcntl.h > > +++ b/include/uapi/linux/fcntl.h > > @@ -49,6 +49,7 @@ > > #define F_SEAL_WRITE 0x0008 /* prevent writes */ > > #define F_SEAL_FUTURE_WRITE 0x0010 /* prevent future writes while = mapped */ > > #define F_SEAL_EXEC 0x0020 /* prevent chmod modifying exec bits */ > > +#define F_SEAL_FUTURE_EXEC 0x0040 /* prevent future executable map= pings */ > > /* (1U << 31) is reserved for signed error codes */ > > > > /* > > diff --git a/mm/memfd.c b/mm/memfd.c > > index 5f5a23c9051d..cfd62454df5e 100644 > > --- a/mm/memfd.c > > +++ b/mm/memfd.c > > @@ -184,6 +184,7 @@ static unsigned int *memfd_file_seals_ptr(struct fi= le *file) > > } > > > > #define F_ALL_SEALS (F_SEAL_SEAL | \ > > + F_SEAL_FUTURE_EXEC |\ > > F_SEAL_EXEC | \ > > F_SEAL_SHRINK | \ > > F_SEAL_GROW | \ > > @@ -357,14 +358,50 @@ static int check_write_seal(unsigned long *vm_fla= gs_ptr) > > return 0; > > } > > > > +static inline bool is_exec_sealed(unsigned int seals) > > +{ > > + return seals & F_SEAL_FUTURE_EXEC; > > +} > > + > > +static int check_exec_seal(unsigned long *vm_flags_ptr) > > +{ > > + unsigned long vm_flags =3D *vm_flags_ptr; > > + unsigned long mask =3D vm_flags & (VM_SHARED | VM_EXEC); > > + > > + /* Executability is not a concern for private mappings. */ > > + if (!(mask & VM_SHARED)) > > + return 0; > > Why is it not a concern for private mappings? > > > + /* > > + * New PROT_EXEC and MAP_SHARED mmaps are not allowed when exec= seal > > + * is active. > > + */ > > + if (mask & VM_EXEC) > > + return -EPERM; > > + > > + /* > > + * Prevent mprotect() from making an exec-sealed mapping execut= able in > > + * the future. > > + */ > > + *vm_flags_ptr &=3D ~VM_MAYEXEC; > > + > > + return 0; > > +} > > + > > int memfd_check_seals_mmap(struct file *file, unsigned long *vm_flags_= ptr) > > { > > int err =3D 0; > > unsigned int *seals_ptr =3D memfd_file_seals_ptr(file); > > unsigned int seals =3D seals_ptr ? *seals_ptr : 0; > > > > - if (is_write_sealed(seals)) > > + if (is_write_sealed(seals)) { > > err =3D check_write_seal(vm_flags_ptr); > > + if (err) > > + return err; > > + } > > + > > + if (is_exec_sealed(seals)) > > + err =3D check_exec_seal(vm_flags_ptr); > > memfd_check_seals_mmap is only for mmap() path, right ? How about the mprotect() path ? i.e. An attacker can first create a RW VMA mapping for the memfd and later mprotect the VMA to be executable. Similar to the check_write_seal call , we might want to block mprotect for write seal as well. > > return err; > > } > > -- > > 2.47.1.613.gc27f4b7a9f-goog > > > > > > >