Received-SPF: pass (google.com: domain of linux-kernel+bounces-75494-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
From: Adrian Ratiu <adrian.ratiu@collabora.com>
To: linux-security-module@vger.kernel.org,
	linux-fsdevel@vger.kernel.org
Cc: linux-kernel@vger.kernel.org,
	kernel@collabora.com,
	Adrian Ratiu <adrian.ratiu@collabora.com>,
	Guenter Roeck <groeck@chromium.org>,
	Doug Anderson <dianders@chromium.org>,
	Mike Frysinger <vapier@chromium.org>
Subject: [PATCH] proc: allow restricting /proc/pid/mem writes
Date: Wed, 21 Feb 2024 23:06:26 +0200
Message-ID: <20240221210626.155534-1-adrian.ratiu@collabora.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

Prior to v2.6.39 write access to /proc/<pid>/mem was restricted,
after which it got allowed in commit 198214a7ee50 ("proc: enable
writing to /proc/pid/mem"). Famous last words from that patch:
"no longer a security hazard". :)

Afterwards exploits appeared started causing drama like [1]. The
/proc/*/mem exploits can be rather sophisticated like [2] which
installed an arbitrary payload from noexec storage into a running
process then exec'd it, which itself could include an ELF loader
to run arbitrary code off noexec storage.

As part of hardening against these types of attacks, distrbutions
can restrict /proc/*/mem to only allow writes when they makes sense,
like in case of debuggers which have ptrace permissions, as they
are able to access memory anyway via PTRACE_POKEDATA and friends.

Dropping the mode bits disables write access for non-root users.
Trying to `chmod` the paths back fails as the kernel rejects it.

For users with CAP_DAC_OVERRIDE (usually just root) we have to
disable the mem_write callback to avoid bypassing the mode bits.

Writes can be used to bypass permissions on memory maps, even if a
memory region is mapped r-x (as is a program's executable pages),
the process can open its own /proc/self/mem file and write to the
pages directly.

Even if seccomp filters block mmap/mprotect calls with W|X perms,
they often cannot block open calls as daemons want to read/write
their own runtime state and seccomp filters cannot check file paths.
Write calls also can't be blocked in general via seccomp.

Since the mem file is part of the dynamic /proc/<pid>/ space, we
can't run chmod once at boot to restrict it (and trying to react
to every process and run chmod doesn't scale, and the kernel no
longer allows chmod on any of these paths).

SELinux could be used with a rule to cover all /proc/*/mem files,
but even then having multiple ways to deny an attack is useful in
case on layer fails.

[1] https://lwn.net/Articles/476947/
[2] https://issues.chromium.org/issues/40089045

Based on an initial patch by Mike Frysinger <vapier@chromium.org>.

Cc: Guenter Roeck <groeck@chromium.org>
Cc: Doug Anderson <dianders@chromium.org>
Signed-off-by: Mike Frysinger <vapier@chromium.org>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
Tested on next-20240220.

I would really like to avoid depending on CONFIG_MEMCG which is
required for the struct mm_stryct "owner" pointer.

Any suggestions how check the ptrace owner without MEMCG?
---
 fs/proc/base.c   | 26 ++++++++++++++++++++++++--
 security/Kconfig | 13 +++++++++++++
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 98a031ac2648..e4d6829c5d1a 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -152,6 +152,12 @@ struct pid_entry {
 		NULL, &proc_pid_attr_operations,	\
 		{ .lsmid = LSMID })
 
+#ifdef CONFIG_SECURITY_PROC_MEM_RESTRICT_WRITES
+# define PROC_PID_MEM_MODE S_IRUSR
+#else
+# define PROC_PID_MEM_MODE (S_IRUSR|S_IWUSR)
+#endif
+
 /*
  * Count the number of hardlinks for the pid_entry table, excluding the .
  * and .. links.
@@ -899,7 +905,24 @@ static ssize_t mem_read(struct file *file, char __user *buf,
 static ssize_t mem_write(struct file *file, const char __user *buf,
 			 size_t count, loff_t *ppos)
 {
+#ifdef CONFIG_SECURITY_PROC_MEM_RESTRICT_WRITES
+	/* Allow processes already ptracing the target process */
+#ifdef CONFIG_MEMCG
+	struct mm_struct *mm = file->private_data;
+
+	rcu_read_lock();
+	if (ptracer_capable(current, mm->user_ns) &&
+	    current == ptrace_parent(mm->owner)) {
+		rcu_read_unlock();
+		return mem_rw(file, (char __user *)buf, count, ppos, 1);
+	}
+	rcu_read_unlock();
+#endif
+
+	return -EACCES;
+#else
 	return mem_rw(file, (char __user*)buf, count, ppos, 1);
+#endif
 }
 
 loff_t mem_lseek(struct file *file, loff_t offset, int orig)
@@ -3281,7 +3303,7 @@ static const struct pid_entry tgid_base_stuff[] = {
 #ifdef CONFIG_NUMA
 	REG("numa_maps",  S_IRUGO, proc_pid_numa_maps_operations),
 #endif
-	REG("mem",        S_IRUSR|S_IWUSR, proc_mem_operations),
+	REG("mem",        PROC_PID_MEM_MODE, proc_mem_operations),
 	LNK("cwd",        proc_cwd_link),
 	LNK("root",       proc_root_link),
 	LNK("exe",        proc_exe_link),
@@ -3631,7 +3653,7 @@ static const struct pid_entry tid_base_stuff[] = {
 #ifdef CONFIG_NUMA
 	REG("numa_maps", S_IRUGO, proc_pid_numa_maps_operations),
 #endif
-	REG("mem",       S_IRUSR|S_IWUSR, proc_mem_operations),
+	REG("mem",       PROC_PID_MEM_MODE, proc_mem_operations),
 	LNK("cwd",       proc_cwd_link),
 	LNK("root",      proc_root_link),
 	LNK("exe",       proc_exe_link),
diff --git a/security/Kconfig b/security/Kconfig
index 412e76f1575d..4082a07a33e5 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -19,6 +19,19 @@ config SECURITY_DMESG_RESTRICT
 
 	  If you are unsure how to answer this question, answer N.
 
+config SECURITY_PROC_MEM_RESTRICT_WRITES
+	bool "Restrict /proc/<pid>/mem write access"
+	default n
+	help
+	  This restricts writes to /proc/<pid>/mem, except when the current
+	  process ptraces the /proc/<pid>/mem task, because a ptracer already
+	  has write access to the tracee memory.
+
+	  Write access to this file allows bypassing memory map permissions,
+	  such as modifying read-only code.
+
+	  If you are unsure how to answer this question, answer N.
+
 config SECURITY
 	bool "Enable different security models"
 	depends on SYSFS
-- 
2.30.2