FAILED: patch "[PATCH] proc: protect ptrace_may_access() with exec_update_lock (FD" failed to apply to 5.10-stable tree
Subsystems (2)
[email protected]
maintainerThe patch below does not apply to the 5.10-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-5.10.y
git checkout FETCH_HEAD
git cherry-pick -x 6255da28d4bb5349fe18e84cb043ccd394eba75d
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2026071447-magnesium-aghast-e41f@gregkh' --subject-prefix 'PATCH 5.10.y' 'HEAD^..'
Possible dependencies:
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 6255da28d4bb5349fe18e84cb043ccd394eba75d Mon Sep 17 00:00:00 2001
From: Jann Horn <[email protected]>
Date: Mon, 18 May 2026 18:35:16 +0200
Subject: [PATCH] proc: protect ptrace_may_access() with exec_update_lock (FD
links)
proc_pid_get_link() and proc_pid_readlink() currently look up the task from the pid once, then do the ptrace access check on that task, then look up the task from the pid a second time to do the actual access. That's racy in several ways.
To fix it, pass the task to the ->proc_get_link() handler, and instead of proc_fd_access_allowed(), introduce a new helper call_proc_get_link() that looks up and locks the task, does the access check, and calls ->proc_get_link().
fs/proc/base.c +47 -72
@@ -218,33 +218,24 @@ static int get_task_root(struct task_struct *task, struct path *root)return result; } -static int proc_cwd_link (struct dentry *dentry,struct path *path) +static int proc_cwd_link (struct dentry *dentry,struct path *path, +struct task_struct *task) { -struct task_struct *task =get_proc_task (d_inode (dentry));int result = -ENOENT ; -if (task) { -task_lock (task); -if (task->fs) { -get_fs_pwd (task->fs, path); - result =0 ; - } -task_unlock (task); -put_task_struct (task); +task_lock (task); +if (task->fs) { +get_fs_pwd (task->fs, path); + result =0 ; } +task_unlock (task);return result; } -static int proc_root_link (struct dentry *dentry,struct path *path) +static int proc_root_link (struct dentry *dentry,struct path *path, +struct task_struct *task) { -struct task_struct *task =get_proc_task (d_inode (dentry)); -int result = -ENOENT ; - -if (task) { - result =get_task_root (task, path); -put_task_struct (task); - } -return result; +return get_task_root (task, path); }/*
@@ -710,23 +701,6 @@ static int proc_pid_syscall(struct seq_file *m, struct pid_namespace *ns,/* Here the fs part begins */ /************************************************************************/ -/* permission checks */ -static bool proc_fd_access_allowed (struct inode *inode) -{ -struct task_struct *task; -bool allowed = false; -/* Allow access to a task's file descriptors if it is us or we -* may use ptrace attach to the process and find out that -* information. -*/ - task =get_proc_task (inode); -if (task) { - allowed =ptrace_may_access (task,PTRACE_MODE_READ_FSCREDS ); -put_task_struct (task); - } -return allowed; -} -int proc_nochmod_setattr (struct mnt_idmap *idmap,struct dentry *dentry,struct iattr *attr) {
@@ -1783,16 +1757,12 @@ static const struct file_operations proc_pid_set_comm_operations = { .release = single_release, }; -static int proc_exe_link (struct dentry *dentry,struct path *exe_path) +static int proc_exe_link (struct dentry *dentry,struct path *exe_path, +struct task_struct *task) { -struct task_struct *task;struct file *exe_file; - task =get_proc_task (d_inode (dentry)); -if (!task) -return -ENOENT ; exe_file =get_task_exe_file (task); -put_task_struct (task);if (exe_file) { *exe_path = exe_file->f_path;path_get (&exe_file->f_path);
@@ -1802,26 +1772,42 @@ static int proc_exe_link(struct dentry *dentry, struct path *exe_path)return -ENOENT ; } +static int call_proc_get_link (struct dentry *dentry,struct inode *inode,struct path *path_out) +{ +struct task_struct *task; +int ret; + + task =get_proc_task (inode); +if (!task) +return -ENOENT ; + ret =down_read_killable (&task->signal->exec_update_lock); +if (ret) + goto out_put_task; +if (!ptrace_may_access (task,PTRACE_MODE_READ_FSCREDS )) { + ret = -EACCES ; + goto out; + } + ret =PROC_I (inode)->op.proc_get_link (dentry, path_out, task); + +out: +up_read (&task->signal->exec_update_lock); +out_put_task: +put_task_struct (task); +return ret; +} +static const char *proc_pid_get_link (struct dentry *dentry,struct inode *inode,struct delayed_call *done) {struct path path; -int error = -EACCES ; +int error;if (!dentry)return ERR_PTR (-ECHILD ); - -/* Are we allowed to snoop on the tasks file descriptors? */ -if (!proc_fd_access_allowed (inode)) - goto out; - - error =PROC_I (inode)->op.proc_get_link (dentry, &path); -if (error) - goto out; - - error =nd_jump_link (&path); -out: + error =call_proc_get_link (dentry, inode, &path); +if (!error) + error =nd_jump_link (&path);return ERR_PTR (error); }
@@ -1855,17 +1841,11 @@ static int proc_pid_readlink(struct dentry * dentry, char __user * buffer, int bstruct inode *inode =d_inode (dentry);struct path path; -/* Are we allowed to snoop on the tasks file descriptors? */ -if (!proc_fd_access_allowed (inode)) - goto out; - - error =PROC_I (inode)->op.proc_get_link (dentry, &path); -if (error) - goto out; - - error =do_proc_readlink (&path, buffer, buflen); -path_put (&path); -out: + error =call_proc_get_link (dentry, inode, &path); +if (!error) { + error =do_proc_readlink (&path, buffer, buflen); +path_put (&path); + }return error; }
@@ -2256,21 +2236,16 @@ static const struct dentry_operations tid_map_files_dentry_operations = { .d_delete = pid_delete_dentry, }; -static int map_files_get_link (struct dentry *dentry,struct path *path) +static int map_files_get_link (struct dentry *dentry,struct path *path, +struct task_struct *task) {unsigned long vm_start, vm_end;struct vm_area_struct *vma; -struct task_struct *task;struct mm_struct *mm;int rc; rc = -ENOENT ; - task =get_proc_task (d_inode (dentry)); -if (!task) - goto out; - mm =get_task_mm (task); -put_task_struct (task);if (!mm) goto out;
fs/proc/fd.c +10 -15
@@ -171,24 +171,19 @@ static const struct dentry_operations tid_fd_dentry_operations = { .d_delete = pid_delete_dentry, }; -static int proc_fd_link (struct dentry *dentry,struct path *path) +static int proc_fd_link (struct dentry *dentry,struct path *path, +struct task_struct *task) { -struct task_struct *task;int ret = -ENOENT ; +unsigned int fd =proc_fd (d_inode (dentry)); +struct file *fd_file; - task =get_proc_task (d_inode (dentry)); -if (task) { -unsigned int fd =proc_fd (d_inode (dentry)); -struct file *fd_file; - - fd_file =fget_task (task, fd); -if (fd_file) { - *path = fd_file->f_path; -path_get (&fd_file->f_path); - ret =0 ; -fput (fd_file); - } -put_task_struct (task); + fd_file =fget_task (task, fd); +if (fd_file) { + *path = fd_file->f_path; +path_get (&fd_file->f_path); + ret =0 ; +fput (fd_file); }return ret;
fs/proc/internal.h +1 -1
@@ -110,7 +110,7 @@ extern struct kmem_cache *proc_dir_entry_cache;void pde_free (struct proc_dir_entry *pde);union proc_op { -int (*proc_get_link)(struct dentry *,struct path *); +int (*proc_get_link)(struct dentry *,struct path *,struct task_struct *);int (*proc_show)(struct seq_file *m,struct pid_namespace *ns,struct pid *pid,struct task_struct *task);
Sasha Levin
maintainerPATCH 1/3 proc: use generic setattr() for /proc/$PID/net
From: Thomas Weißschuh <[email protected]>
[ Upstream commit 18e66ae67673f2c8df6f02428798b1355691f2a9 ]
All other files in /proc/$PID/ use proc_setattr().
Not using it allows the usage of chmod() on /proc/$PID/net, even on other processes owned by the same user.
The same would probably also be true for other attributes to be changed.
As this technically represents an ABI change it is not marked for stable so any unlikely regressions are caught during a full release cycle.
Message-Id: <[email protected]>
Stable-dep-of: 6255da28d4bb ("proc: protect ptrace_may_access() with exec_update_lock (FD links)")
--- fs/proc/proc_net.c | 1 + 1 file changed, 1 insertion(+)
fs/proc/proc_net.c +1 -0
@@ -310,6 +310,7 @@ static int proc_tgid_net_getattr(const struct path *path, struct kstat *stat,const struct inode_operations proc_net_inode_operations = { .lookup = proc_tgid_net_lookup, .getattr = proc_tgid_net_getattr, + .setattr = proc_setattr, };static int proc_tgid_net_readdir (struct file *file,struct dir_context *ctx)
Signature · 1 line
2.53.0
Sasha Levin
maintainerPATCH 2/3 proc: rename proc_setattr to proc_nochmod_setattr
From: Christoph Hellwig <[email protected]>
[ Upstream commit 690005b0b1e6b567c88b7790e6d90d4d6c9e09cc ]
What is currently proc_setattr is a special version added after the more general procfs ->seattr in commit 6d76fa58b050 ("Don't allow chmod() on the /proc/<pid>/ files"). Give it a name that reflects that to free the proc_setattr name and better describe what is doing.
Stable-dep-of: 6255da28d4bb ("proc: protect ptrace_may_access() with exec_update_lock (FD links)")
--- fs/proc/base.c | 22 +++++++++++----------- fs/proc/fd.c | 4 ++-- fs/proc/internal.h | 2 +- fs/proc/namespaces.c | 4 ++-- fs/proc/proc_net.c | 2 +- 5 files changed, 17 insertions(+), 17 deletions(-)
fs/proc/base.c +11 -11
@@ -720,7 +720,7 @@ static int proc_fd_access_allowed(struct inode *inode)return allowed; } -int proc_setattr (struct dentry *dentry,struct iattr *attr) +int proc_nochmod_setattr (struct dentry *dentry,struct iattr *attr) {int error;struct inode *inode =d_inode (dentry);
@@ -792,7 +792,7 @@ static int proc_pid_permission(struct inode *inode, int mask)static const struct inode_operations proc_def_inode_operations = { - .setattr = proc_setattr, + .setattr = proc_nochmod_setattr, };static int proc_single_show (struct seq_file *m,void *v)
@@ -1863,7 +1863,7 @@ static int proc_pid_readlink(struct dentry * dentry, char __user * buffer, int bconst struct inode_operations proc_pid_link_inode_operations = { .readlink = proc_pid_readlink, .get_link = proc_pid_get_link, - .setattr = proc_setattr, + .setattr = proc_nochmod_setattr, };
@@ -2311,7 +2311,7 @@ proc_map_files_get_link(struct dentry *dentry,static const struct inode_operations proc_map_files_link_inode_operations = { .readlink = proc_pid_readlink, .get_link = proc_map_files_get_link, - .setattr = proc_setattr, + .setattr = proc_nochmod_setattr, }; static struct dentry *
@@ -2390,7 +2390,7 @@ static struct dentry *proc_map_files_lookup(struct inode *dir,static const struct inode_operations proc_map_files_inode_operations = { .lookup = proc_map_files_lookup, .permission = proc_fd_permission, - .setattr = proc_setattr, + .setattr = proc_nochmod_setattr, }; static int
@@ -2882,7 +2882,7 @@ static struct dentry *proc_##LSM##_attr_dir_lookup(struct inode *dir, \ static const struct inode_operations proc_##LSM##_attr_dir_inode_ops = { \ .lookup = proc_##LSM##_attr_dir_lookup, \ .getattr = pid_getattr, \ - .setattr = proc_setattr, \ + .setattr = proc_nochmod_setattr, \ }#ifdef CONFIG_SECURITY_SMACK
@@ -2941,7 +2941,7 @@ static struct dentry *proc_attr_dir_lookup(struct inode *dir,static const struct inode_operations proc_attr_dir_inode_operations = { .lookup = proc_attr_dir_lookup, .getattr = pid_getattr, - .setattr = proc_setattr, + .setattr = proc_nochmod_setattr, };#endif
@@ -3395,7 +3395,7 @@ static struct dentry *proc_tgid_base_lookup(struct inode *dir, struct dentry *destatic const struct inode_operations proc_tgid_base_inode_operations = { .lookup = proc_tgid_base_lookup, .getattr = pid_getattr, - .setattr = proc_setattr, + .setattr = proc_nochmod_setattr, .permission = proc_pid_permission, };
@@ -3594,7 +3594,7 @@ static int proc_tid_comm_permission(struct inode *inode, int mask) }static const struct inode_operations proc_tid_comm_inode_operations = { - .setattr = proc_setattr, + .setattr = proc_nochmod_setattr, .permission = proc_tid_comm_permission, };
@@ -3718,7 +3718,7 @@ static const struct file_operations proc_tid_base_operations = {static const struct inode_operations proc_tid_base_inode_operations = { .lookup = proc_tid_base_lookup, .getattr = pid_getattr, - .setattr = proc_setattr, + .setattr = proc_nochmod_setattr, }; static struct dentry *proc_task_instantiate(struct dentry *dentry,
@@ -3913,7 +3913,7 @@ static int proc_task_getattr(const struct path *path, struct kstat *stat,static const struct inode_operations proc_task_inode_operations = { .lookup = proc_task_lookup, .getattr = proc_task_getattr, - .setattr = proc_setattr, + .setattr = proc_nochmod_setattr, .permission = proc_pid_permission, };
fs/proc/fd.c +2 -2
@@ -297,7 +297,7 @@ int proc_fd_permission(struct inode *inode, int mask)const struct inode_operations proc_fd_inode_operations = { .lookup = proc_lookupfd, .permission = proc_fd_permission, - .setattr = proc_setattr, + .setattr = proc_nochmod_setattr, }; static struct dentry *proc_fdinfo_instantiate(struct dentry *dentry,
@@ -335,7 +335,7 @@ static int proc_readfdinfo(struct file *file, struct dir_context *ctx)const struct inode_operations proc_fdinfo_inode_operations = { .lookup = proc_lookupfdinfo, - .setattr = proc_setattr, + .setattr = proc_nochmod_setattr, };const struct file_operations proc_fdinfo_operations = {
fs/proc/internal.h +1 -1
@@ -177,7 +177,7 @@ extern int proc_pid_statm(struct seq_file *, struct pid_namespace *,*/ extern const struct dentry_operations pid_dentry_operations;extern int pid_getattr (const struct path *,struct kstat *,u32 ,unsigned int ); -extern int proc_setattr (struct dentry *,struct iattr *); +int proc_nochmod_setattr (struct dentry *dentry,struct iattr *attr);extern void proc_pid_evict_inode (struct proc_inode *);extern struct inode *proc_pid_make_inode (struct super_block *,struct task_struct *,umode_t );extern void pid_update_inode (struct task_struct *,struct inode *);
fs/proc/namespaces.c +2 -2
@@ -92,7 +92,7 @@ static int proc_ns_readlink(struct dentry *dentry, char __user *buffer, int buflstatic const struct inode_operations proc_ns_link_inode_operations = { .readlink = proc_ns_readlink, .get_link = proc_ns_get_link, - .setattr = proc_setattr, + .setattr = proc_nochmod_setattr, }; static struct dentry *proc_ns_instantiate(struct dentry *dentry,
@@ -179,5 +179,5 @@ static struct dentry *proc_ns_dir_lookup(struct inode *dir,const struct inode_operations proc_ns_dir_inode_operations = { .lookup = proc_ns_dir_lookup, .getattr = pid_getattr, - .setattr = proc_setattr, + .setattr = proc_nochmod_setattr, };
fs/proc/proc_net.c +1 -1
@@ -310,7 +310,7 @@ static int proc_tgid_net_getattr(const struct path *path, struct kstat *stat,const struct inode_operations proc_net_inode_operations = { .lookup = proc_tgid_net_lookup, .getattr = proc_tgid_net_getattr, - .setattr = proc_setattr, + .setattr = proc_nochmod_setattr, };static int proc_tgid_net_readdir (struct file *file,struct dir_context *ctx)
Signature · 1 line
2.53.0
Sasha Levin
maintainerPATCH 3/3 proc: protect ptrace_may_access() with exec_update_lock (FD links)
In reply to Sasha LevinFrom: Jann Horn <[email protected]>
[ Upstream commit 6255da28d4bb5349fe18e84cb043ccd394eba75d ]
proc_pid_get_link() and proc_pid_readlink() currently look up the task from the pid once, then do the ptrace access check on that task, then look up the task from the pid a second time to do the actual access. That's racy in several ways.
To fix it, pass the task to the ->proc_get_link() handler, and instead of proc_fd_access_allowed(), introduce a new helper call_proc_get_link() that looks up and locks the task, does the access check, and calls ->proc_get_link().
--- fs/proc/base.c | 119 ++++++++++++++++++--------------------------- fs/proc/fd.c | 27 +++++----- fs/proc/internal.h | 2 +- 3 files changed, 59 insertions(+), 89 deletions(-)
fs/proc/base.c +47 -72
@@ -217,33 +217,24 @@ static int get_task_root(struct task_struct *task, struct path *root)return result; } -static int proc_cwd_link (struct dentry *dentry,struct path *path) +static int proc_cwd_link (struct dentry *dentry,struct path *path, +struct task_struct *task) { -struct task_struct *task =get_proc_task (d_inode (dentry));int result = -ENOENT ; -if (task) { -task_lock (task); -if (task->fs) { -get_fs_pwd (task->fs, path); - result =0 ; - } -task_unlock (task); -put_task_struct (task); +task_lock (task); +if (task->fs) { +get_fs_pwd (task->fs, path); + result =0 ; } +task_unlock (task);return result; } -static int proc_root_link (struct dentry *dentry,struct path *path) +static int proc_root_link (struct dentry *dentry,struct path *path, +struct task_struct *task) { -struct task_struct *task =get_proc_task (d_inode (dentry)); -int result = -ENOENT ; - -if (task) { - result =get_task_root (task, path); -put_task_struct (task); - } -return result; +return get_task_root (task, path); }/*
@@ -703,23 +694,6 @@ static int proc_pid_syscall(struct seq_file *m, struct pid_namespace *ns,/* Here the fs part begins */ /************************************************************************/ -/* permission checks */ -static int proc_fd_access_allowed (struct inode *inode) -{ -struct task_struct *task; -int allowed =0 ; -/* Allow access to a task's file descriptors if it is us or we -* may use ptrace attach to the process and find out that -* information. -*/ - task =get_proc_task (inode); -if (task) { - allowed =ptrace_may_access (task,PTRACE_MODE_READ_FSCREDS ); -put_task_struct (task); - } -return allowed; -} -int proc_nochmod_setattr (struct dentry *dentry,struct iattr *attr) {int error;
@@ -1774,16 +1748,12 @@ static const struct file_operations proc_pid_set_comm_operations = { .release = single_release, }; -static int proc_exe_link (struct dentry *dentry,struct path *exe_path) +static int proc_exe_link (struct dentry *dentry,struct path *exe_path, +struct task_struct *task) { -struct task_struct *task;struct file *exe_file; - task =get_proc_task (d_inode (dentry)); -if (!task) -return -ENOENT ; exe_file =get_task_exe_file (task); -put_task_struct (task);if (exe_file) { *exe_path = exe_file->f_path;path_get (&exe_file->f_path);
@@ -1793,26 +1763,42 @@ static int proc_exe_link(struct dentry *dentry, struct path *exe_path)return -ENOENT ; } +static int call_proc_get_link (struct dentry *dentry,struct inode *inode,struct path *path_out) +{ +struct task_struct *task; +int ret; + + task =get_proc_task (inode); +if (!task) +return -ENOENT ; + ret =down_read_killable (&task->signal->exec_update_lock); +if (ret) + goto out_put_task; +if (!ptrace_may_access (task,PTRACE_MODE_READ_FSCREDS )) { + ret = -EACCES ; + goto out; + } + ret =PROC_I (inode)->op.proc_get_link (dentry, path_out, task); + +out: +up_read (&task->signal->exec_update_lock); +out_put_task: +put_task_struct (task); +return ret; +} +static const char *proc_pid_get_link (struct dentry *dentry,struct inode *inode,struct delayed_call *done) {struct path path; -int error = -EACCES ; +int error;if (!dentry)return ERR_PTR (-ECHILD ); - -/* Are we allowed to snoop on the tasks file descriptors? */ -if (!proc_fd_access_allowed (inode)) - goto out; - - error =PROC_I (inode)->op.proc_get_link (dentry, &path); -if (error) - goto out; - - error =nd_jump_link (&path); -out: + error =call_proc_get_link (dentry, inode, &path); +if (!error) + error =nd_jump_link (&path);return ERR_PTR (error); }
@@ -1846,17 +1832,11 @@ static int proc_pid_readlink(struct dentry * dentry, char __user * buffer, int bstruct inode *inode =d_inode (dentry);struct path path; -/* Are we allowed to snoop on the tasks file descriptors? */ -if (!proc_fd_access_allowed (inode)) - goto out; - - error =PROC_I (inode)->op.proc_get_link (dentry, &path); -if (error) - goto out; - - error =do_proc_readlink (&path, buffer, buflen); -path_put (&path); -out: + error =call_proc_get_link (dentry, inode, &path); +if (!error) { + error =do_proc_readlink (&path, buffer, buflen); +path_put (&path); + }return error; }
@@ -2242,21 +2222,16 @@ static const struct dentry_operations tid_map_files_dentry_operations = { .d_delete = pid_delete_dentry, }; -static int map_files_get_link (struct dentry *dentry,struct path *path) +static int map_files_get_link (struct dentry *dentry,struct path *path, +struct task_struct *task) {unsigned long vm_start, vm_end;struct vm_area_struct *vma; -struct task_struct *task;struct mm_struct *mm;int rc; rc = -ENOENT ; - task =get_proc_task (d_inode (dentry)); -if (!task) - goto out; - mm =get_task_mm (task); -put_task_struct (task);if (!mm) goto out;
fs/proc/fd.c +11 -16
@@ -140,24 +140,19 @@ static const struct dentry_operations tid_fd_dentry_operations = { .d_delete = pid_delete_dentry, }; -static int proc_fd_link (struct dentry *dentry,struct path *path) +static int proc_fd_link (struct dentry *dentry,struct path *path, +struct task_struct *task) { -struct task_struct *task;int ret = -ENOENT ; - - task =get_proc_task (d_inode (dentry)); -if (task) { -unsigned int fd =proc_fd (d_inode (dentry)); -struct file *fd_file; - - fd_file =fget_task (task, fd); -if (fd_file) { - *path = fd_file->f_path; -path_get (&fd_file->f_path); - ret =0 ; -fput (fd_file); - } -put_task_struct (task); +unsigned int fd =proc_fd (d_inode (dentry)); +struct file *fd_file; + + fd_file =fget_task (task, fd); +if (fd_file) { + *path = fd_file->f_path; +path_get (&fd_file->f_path); + ret =0 ; +fput (fd_file); }return ret;
fs/proc/internal.h +1 -1
@@ -97,7 +97,7 @@ extern struct kmem_cache *proc_dir_entry_cache;void pde_free (struct proc_dir_entry *pde);union proc_op { -int (*proc_get_link)(struct dentry *,struct path *); +int (*proc_get_link)(struct dentry *,struct path *,struct task_struct *);int (*proc_show)(struct seq_file *m,struct pid_namespace *ns,struct pid *pid,struct task_struct *task);
Signature · 1 line
2.53.0