Re: [PATCH] proc: add config to block FOLL_FORCE in mem writes

Linus Torvalds
2024-07-17T18:17:16Z

[-- Attachment #1: Type: text/plain, Size: 1585 bytes --]

On Wed, 17 Jul 2024 at 10:23, Kees Cook <kees@kernel.org> wrote:
>
> For this to be available for general distros, I still want to have a
> bootparam to control this, otherwise this mitigation will never see much
> testing as most kernel deployments don't build their own kernels. A
> simple __ro_after_init variable can be used.

Oh, btw, I looked at the FOLL_FORCE back in 2017 when we did this:

8ee74a91ac30 ("proc: try to remove use of FOLL_FORCE entirely")

and then we had to undo that with

f511c0b17b08 (""Yes, people use FOLL_FORCE ;)"")

but at the time I also had an experimental patch that worked for me,
but I seem to have only sent that out in private to the people
involved with the original issue.

And then that whole discussion petered out, and nothing happened.

But maybe we can try again.

In particular, while people piped up about other uses (see the quotes
in that commit f511c0b17b08) they were fairly rare and specialized.

The one *common* use was gdb.

But my old diff from years ago mostly still applies, so I resurrected it.

It basically restricts FOLL_FORCE to just ptracers.

That's *not* good for some of the people that piped up back when (eg
Julia JIT), but it might be a more palatable halfway state.

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().

I guess that function should be renamed too, I only did a minimal
"forward-port an old patch" thing.

Linus

[-- Attachment #2: foll_force.patch --]
[-- Type: text/x-patch, Size: 1081 bytes --]

fs/proc/base.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 72a1acd03675..1b646cb96509 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -835,6 +835,18 @@ static int mem_open(struct inode *inode, struct file *file)
return ret;
}

+static bool proc_is_ptracing(struct file *file, struct mm_struct *mm)
+{
+ 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;
+}
+
static ssize_t mem_rw(struct file *file, char __user *buf,
size_t count, loff_t *ppos, int write)
{
@@ -855,7 +867,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_is_ptracing(file, mm))
+ flags |= FOLL_FORCE;

while (count > 0) {
size_t this_len = min_t(size_t, count, PAGE_SIZE);