On Wed, 17 Jul 2024 at 15:24, Kees Cook <kees@xxxxxxxxxx> wrote:
>
> > In particular, this patch would make it easy to make that
> > SECURITY_PROC_MEM_RESTRICT_FOLL_FORCE config option be a "choice"
> > where you pick "never, ptrace, always" by just changing the rules in
> > proc_is_ptracing().
>
> So the original patch could be reduced to just the single tristate option
> instead of 3 tristates? I think that would be a decent middle ground,
> and IIUC, will still provide the coverage Chrome OS is looking for[1].
So here's what I kind of think might be ok.
ENTIRELY UNTESTED! This is more of a "look, something like this,
perhaps" patch than a real one.
If somebody tests this, and it is ok for Chrome OS, you can consider
this signed-off-on, but only with actual testing. I might have gotten
something hroribly wrong.
Linus
fs/proc/base.c | 22 +++++++++++++++++++++-
security/Kconfig | 32 ++++++++++++++++++++++++++++++++
2 files changed, 53 insertions(+), 1 deletion(-)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 72a1acd03675..fbe9a96c2d98 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -835,6 +835,24 @@ static int mem_open(struct inode *inode, struct file *file)
return ret;
}
+static bool proc_mem_foll_force(struct file *file, struct mm_struct *mm)
+{
+#if defined(CONFIG_PROC_MEM_NO_FORCE)
+ return false;
+#elif defined(CONFIG_PROC_MEM_FORCE_PTRACE)
+ bool ptrace_active = false;
+ struct task_struct *task = get_proc_task(file_inode(file));
+
+ if (task) {
+ ptrace_active = task->ptrace && task->mm == mm && task->parent == current;
+ put_task_struct(task);
+ }
+ return ptrace_active;
+#else
+ return true;
+#endif
+}
+
static ssize_t mem_rw(struct file *file, char __user *buf,
size_t count, loff_t *ppos, int write)
{
@@ -855,7 +873,9 @@ static ssize_t mem_rw(struct file *file, char __user *buf,
if (!mmget_not_zero(mm))
goto free;
- flags = FOLL_FORCE | (write ? FOLL_WRITE : 0);
+ flags = write ? FOLL_WRITE : 0;
+ if (proc_mem_foll_force(file, mm))
+ flags |= FOLL_FORCE;
while (count > 0) {
size_t this_len = min_t(size_t, count, PAGE_SIZE);
diff --git a/security/Kconfig b/security/Kconfig
index 412e76f1575d..b201ae3feeab 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -19,6 +19,38 @@ config SECURITY_DMESG_RESTRICT
If you are unsure how to answer this question, answer N.
+choice
+ prompt "Allow /proc/pid/mem access override"
+ default PROC_PID_MEM_ALWAYS_FORCE
+ help
+ Traditionally /proc/pid/mem allows users to override memory
+ permissions for users like ptrace, assuming they have ptrace
+ capability.
+
+ This allows people to limit that - either never override, or
+ require actual active ptrace attachment.
+
+ Defaults to the traditional behavior (for now)
+
+config PROC_PID_MEM_ALWAYS_FORCE
+ bool "Traditional /proc/pid/mem behavior"
+ help
+ This allows /proc/pid/mem accesses to override memory mapping
+ permissions if you have ptrace access rights.
+
+config CONFIG_PROC_MEM_FORCE_PTRACE
+ bool "Require active ptrace() use for access override"
+ help
+ This allows /proc/pid/mem accesses to override memory mapping
+ permissions for active ptracers like gdb.
+
+config CONFIG_PROC_MEM_NO_FORCE
+ bool "Never"
+ help
+ Never override memory mapping permissions
+
+endchoice
+
config SECURITY
bool "Enable different security models"
depends on SYSFS
