* [PATCH v2 4.9 01/10] clone: add CLONE_PIDFD
2021-01-07 7:52 [PATCH v2 4.9 00/10] fix a race in release_task when flushing the dentry Wen Yang
@ 2021-01-07 7:52 ` Wen Yang
2021-01-07 7:52 ` [PATCH v2 4.9 02/10] pidfd: add polling support Wen Yang
` (9 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: Wen Yang @ 2021-01-07 7:52 UTC (permalink / raw)
To: Greg Kroah-Hartman, Sasha Levin
Cc: Xunlei Pang, linux-kernel, Christian Brauner, Linus Torvalds,
Jann Horn, Oleg Nesterov, Arnd Bergmann, Eric W. Biederman,
Kees Cook, Thomas Gleixner, David Howells,
Michael Kerrisk (man-pages), Andy Lutomirsky, Andrew Morton,
Aleksa Sarai, Al Viro, stable, Wen Yang
From: Christian Brauner <christian@brauner.io>
[ Upstream commit b3e5838252665ee4cfa76b82bdf1198dca81e5be ]
This patchset makes it possible to retrieve pid file descriptors at
process creation time by introducing the new flag CLONE_PIDFD to the
clone() system call. Linus originally suggested to implement this as a
new flag to clone() instead of making it a separate system call. As
spotted by Linus, there is exactly one bit for clone() left.
CLONE_PIDFD creates file descriptors based on the anonymous inode
implementation in the kernel that will also be used to implement the new
mount api. They serve as a simple opaque handle on pids. Logically,
this makes it possible to interpret a pidfd differently, narrowing or
widening the scope of various operations (e.g. signal sending). Thus, a
pidfd cannot just refer to a tgid, but also a tid, or in theory - given
appropriate flag arguments in relevant syscalls - a process group or
session. A pidfd does not represent a privilege. This does not imply it
cannot ever be that way but for now this is not the case.
A pidfd comes with additional information in fdinfo if the kernel supports
procfs. The fdinfo file contains the pid of the process in the callers
pid namespace in the same format as the procfs status file, i.e. "Pid:\t%d".
As suggested by Oleg, with CLONE_PIDFD the pidfd is returned in the
parent_tidptr argument of clone. This has the advantage that we can
give back the associated pid and the pidfd at the same time.
To remove worries about missing metadata access this patchset comes with
a sample program that illustrates how a combination of CLONE_PIDFD, and
pidfd_send_signal() can be used to gain race-free access to process
metadata through /proc/<pid>. The sample program can easily be
translated into a helper that would be suitable for inclusion in libc so
that users don't have to worry about writing it themselves.
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Christian Brauner <christian@brauner.io>
Co-developed-by: Jann Horn <jannh@google.com>
Signed-off-by: Jann Horn <jannh@google.com>
Reviewed-by: Oleg Nesterov <oleg@redhat.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: David Howells <dhowells@redhat.com>
Cc: "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>
Cc: Andy Lutomirsky <luto@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Aleksa Sarai <cyphar@cyphar.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: <stable@vger.kernel.org> # 4.9.x
(clone: fix up cherry-pick conflicts for b3e583825266)
Signed-off-by: Wen Yang <wenyang@linux.alibaba.com>
---
include/linux/pid.h | 1 +
include/uapi/linux/sched.h | 1 +
kernel/fork.c | 105 +++++++++++++++++++++++++++++++++++++++++++--
3 files changed, 103 insertions(+), 4 deletions(-)
diff --git a/include/linux/pid.h b/include/linux/pid.h
index 97b745d..7599a78 100644
--- a/include/linux/pid.h
+++ b/include/linux/pid.h
@@ -73,6 +73,7 @@ struct pid_link
struct hlist_node node;
struct pid *pid;
};
+extern const struct file_operations pidfd_fops;
static inline struct pid *get_pid(struct pid *pid)
{
diff --git a/include/uapi/linux/sched.h b/include/uapi/linux/sched.h
index 5f0fe01..ed6e31d 100644
--- a/include/uapi/linux/sched.h
+++ b/include/uapi/linux/sched.h
@@ -9,6 +9,7 @@
#define CLONE_FS 0x00000200 /* set if fs info shared between processes */
#define CLONE_FILES 0x00000400 /* set if open files shared between processes */
#define CLONE_SIGHAND 0x00000800 /* set if signal handlers and blocked signals shared */
+#define CLONE_PIDFD 0x00001000 /* set if a pidfd should be placed in parent */
#define CLONE_PTRACE 0x00002000 /* set if we want to let tracing continue on the child too */
#define CLONE_VFORK 0x00004000 /* set if the parent wants the child to wake it up on mm_release */
#define CLONE_PARENT 0x00008000 /* set if we want to have the same parent as the cloner */
diff --git a/kernel/fork.c b/kernel/fork.c
index b64efec..4249f60 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -11,6 +11,7 @@
* management can be a bitch. See 'mm/memory.c': 'copy_page_range()'
*/
+#include <linux/anon_inodes.h>
#include <linux/slab.h>
#include <linux/init.h>
#include <linux/unistd.h>
@@ -1460,6 +1461,58 @@ static void posix_cpu_timers_init(struct task_struct *tsk)
task->pids[type].pid = pid;
}
+static int pidfd_release(struct inode *inode, struct file *file)
+{
+ struct pid *pid = file->private_data;
+
+ file->private_data = NULL;
+ put_pid(pid);
+ return 0;
+}
+
+#ifdef CONFIG_PROC_FS
+static void pidfd_show_fdinfo(struct seq_file *m, struct file *f)
+{
+ struct pid_namespace *ns = file_inode(m->file)->i_sb->s_fs_info;
+ struct pid *pid = f->private_data;
+
+ seq_put_decimal_ull(m, "Pid:\t", pid_nr_ns(pid, ns));
+ seq_putc(m, '\n');
+}
+#endif
+
+const struct file_operations pidfd_fops = {
+ .release = pidfd_release,
+#ifdef CONFIG_PROC_FS
+ .show_fdinfo = pidfd_show_fdinfo,
+#endif
+};
+
+/**
+ * pidfd_create() - Create a new pid file descriptor.
+ *
+ * @pid: struct pid that the pidfd will reference
+ *
+ * This creates a new pid file descriptor with the O_CLOEXEC flag set.
+ *
+ * Note, that this function can only be called after the fd table has
+ * been unshared to avoid leaking the pidfd to the new process.
+ *
+ * Return: On success, a cloexec pidfd is returned.
+ * On error, a negative errno number will be returned.
+ */
+static int pidfd_create(struct pid *pid)
+{
+ int fd;
+
+ fd = anon_inode_getfd("[pidfd]", &pidfd_fops, get_pid(pid),
+ O_RDWR | O_CLOEXEC);
+ if (fd < 0)
+ put_pid(pid);
+
+ return fd;
+}
+
/*
* This creates a new process as a copy of the old one,
* but does not actually start it yet.
@@ -1472,13 +1525,14 @@ static __latent_entropy struct task_struct *copy_process(
unsigned long clone_flags,
unsigned long stack_start,
unsigned long stack_size,
+ int __user *parent_tidptr,
int __user *child_tidptr,
struct pid *pid,
int trace,
unsigned long tls,
int node)
{
- int retval;
+ int pidfd = -1, retval;
struct task_struct *p;
if ((clone_flags & (CLONE_NEWNS|CLONE_FS)) == (CLONE_NEWNS|CLONE_FS))
@@ -1526,6 +1580,30 @@ static __latent_entropy struct task_struct *copy_process(
retval = security_task_create(clone_flags);
if (retval)
goto fork_out;
+ if (clone_flags & CLONE_PIDFD) {
+ int reserved;
+
+ /*
+ * - CLONE_PARENT_SETTID is useless for pidfds and also
+ * parent_tidptr is used to return pidfds.
+ * - CLONE_DETACHED is blocked so that we can potentially
+ * reuse it later for CLONE_PIDFD.
+ * - CLONE_THREAD is blocked until someone really needs it.
+ */
+ if (clone_flags &
+ (CLONE_DETACHED | CLONE_PARENT_SETTID | CLONE_THREAD))
+ return ERR_PTR(-EINVAL);
+
+ /*
+ * Verify that parent_tidptr is sane so we can potentially
+ * reuse it later.
+ */
+ if (get_user(reserved, parent_tidptr))
+ return ERR_PTR(-EFAULT);
+
+ if (reserved != 0)
+ return ERR_PTR(-EINVAL);
+ }
retval = -ENOMEM;
p = dup_task_struct(current, node);
@@ -1703,6 +1781,22 @@ static __latent_entropy struct task_struct *copy_process(
}
}
+ /*
+ * This has to happen after we've potentially unshared the file
+ * descriptor table (so that the pidfd doesn't leak into the child
+ * if the fd table isn't shared).
+ */
+ if (clone_flags & CLONE_PIDFD) {
+ retval = pidfd_create(pid);
+ if (retval < 0)
+ goto bad_fork_free_pid;
+
+ pidfd = retval;
+ retval = put_user(pidfd, parent_tidptr);
+ if (retval)
+ goto bad_fork_put_pidfd;
+ }
+
#ifdef CONFIG_BLOCK
p->plug = NULL;
#endif
@@ -1758,7 +1852,7 @@ static __latent_entropy struct task_struct *copy_process(
*/
retval = cgroup_can_fork(p);
if (retval)
- goto bad_fork_free_pid;
+ goto bad_fork_put_pidfd;
/*
* From this point on we must avoid any synchronous user-space
@@ -1869,6 +1963,9 @@ static __latent_entropy struct task_struct *copy_process(
spin_unlock(¤t->sighand->siglock);
write_unlock_irq(&tasklist_lock);
cgroup_cancel_fork(p);
+bad_fork_put_pidfd:
+ if (clone_flags & CLONE_PIDFD)
+ __close_fd(current->files, pidfd);
bad_fork_free_pid:
threadgroup_change_end(current);
if (pid != &init_struct_pid)
@@ -1928,7 +2025,7 @@ static inline void init_idle_pids(struct pid_link *links)
struct task_struct *fork_idle(int cpu)
{
struct task_struct *task;
- task = copy_process(CLONE_VM, 0, 0, NULL, &init_struct_pid, 0, 0,
+ task = copy_process(CLONE_VM, 0, 0, NULL, NULL, &init_struct_pid, 0, 0,
cpu_to_node(cpu));
if (!IS_ERR(task)) {
init_idle_pids(task->pids);
@@ -1973,7 +2070,7 @@ long _do_fork(unsigned long clone_flags,
trace = 0;
}
- p = copy_process(clone_flags, stack_start, stack_size,
+ p = copy_process(clone_flags, stack_start, stack_size, parent_tidptr,
child_tidptr, NULL, trace, tls, NUMA_NO_NODE);
add_latent_entropy();
/*
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v2 4.9 02/10] pidfd: add polling support
2021-01-07 7:52 [PATCH v2 4.9 00/10] fix a race in release_task when flushing the dentry Wen Yang
2021-01-07 7:52 ` [PATCH v2 4.9 01/10] clone: add CLONE_PIDFD Wen Yang
@ 2021-01-07 7:52 ` Wen Yang
2021-01-07 7:52 ` [PATCH v2 4.9 03/10] proc: Pass file mode to proc_pid_make_inode Wen Yang
` (8 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: Wen Yang @ 2021-01-07 7:52 UTC (permalink / raw)
To: Greg Kroah-Hartman, Sasha Levin
Cc: Xunlei Pang, linux-kernel, Joel Fernandes (Google),
Andy Lutomirski, Steven Rostedt, Daniel Colascione, Jann Horn,
Tim Murray, Jonathan Kowalski, Linus Torvalds, Al Viro, Kees Cook,
David Howells, Oleg Nesterov, kernel-team, Christian Brauner,
stable, Wen Yang
From: "Joel Fernandes (Google)" <joel@joelfernandes.org>
[ Upstream commit b53b0b9d9a613c418057f6cb921c2f40a6f78c24 ]
This patch adds polling support to pidfd.
Android low memory killer (LMK) needs to know when a process dies once
it is sent the kill signal. It does so by checking for the existence of
/proc/pid which is both racy and slow. For example, if a PID is reused
between when LMK sends a kill signal and checks for existence of the
PID, since the wrong PID is now possibly checked for existence.
Using the polling support, LMK will be able to get notified when a process
exists in race-free and fast way, and allows the LMK to do other things
(such as by polling on other fds) while awaiting the process being killed
to die.
For notification to polling processes, we follow the same existing
mechanism in the kernel used when the parent of the task group is to be
notified of a child's death (do_notify_parent). This is precisely when the
tasks waiting on a poll of pidfd are also awakened in this patch.
We have decided to include the waitqueue in struct pid for the following
reasons:
1. The wait queue has to survive for the lifetime of the poll. Including
it in task_struct would not be option in this case because the task can
be reaped and destroyed before the poll returns.
2. By including the struct pid for the waitqueue means that during
de_thread(), the new thread group leader automatically gets the new
waitqueue/pid even though its task_struct is different.
Appropriate test cases are added in the second patch to provide coverage of
all the cases the patch is handling.
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Daniel Colascione <dancol@google.com>
Cc: Jann Horn <jannh@google.com>
Cc: Tim Murray <timmurray@google.com>
Cc: Jonathan Kowalski <bl0pbl33p@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Kees Cook <keescook@chromium.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: kernel-team@android.com
Reviewed-by: Oleg Nesterov <oleg@redhat.com>
Co-developed-by: Daniel Colascione <dancol@google.com>
Signed-off-by: Daniel Colascione <dancol@google.com>
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
Signed-off-by: Christian Brauner <christian@brauner.io>
Cc: <stable@vger.kernel.org> # 4.9.x
(pidfd: fix up cherry-pick conflicts for b53b0b9d9a61)
Signed-off-by: Wen Yang <wenyang@linux.alibaba.com>
---
include/linux/pid.h | 3 +++
kernel/fork.c | 26 ++++++++++++++++++++++++++
kernel/pid.c | 2 ++
kernel/signal.c | 11 +++++++++++
4 files changed, 42 insertions(+)
diff --git a/include/linux/pid.h b/include/linux/pid.h
index 7599a78..f5552ba 100644
--- a/include/linux/pid.h
+++ b/include/linux/pid.h
@@ -2,6 +2,7 @@
#define _LINUX_PID_H
#include <linux/rcupdate.h>
+#include <linux/wait.h>
enum pid_type
{
@@ -62,6 +63,8 @@ struct pid
unsigned int level;
/* lists of tasks that use this pid */
struct hlist_head tasks[PIDTYPE_MAX];
+ /* wait queue for pidfd notifications */
+ wait_queue_head_t wait_pidfd;
struct rcu_head rcu;
struct upid numbers[1];
};
diff --git a/kernel/fork.c b/kernel/fork.c
index 4249f60..e3a4a14 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1481,8 +1481,34 @@ static void pidfd_show_fdinfo(struct seq_file *m, struct file *f)
}
#endif
+/*
+ * Poll support for process exit notification.
+ */
+static unsigned int pidfd_poll(struct file *file, struct poll_table_struct *pts)
+{
+ struct task_struct *task;
+ struct pid *pid = file->private_data;
+ int poll_flags = 0;
+
+ poll_wait(file, &pid->wait_pidfd, pts);
+
+ rcu_read_lock();
+ task = pid_task(pid, PIDTYPE_PID);
+ /*
+ * Inform pollers only when the whole thread group exits.
+ * If the thread group leader exits before all other threads in the
+ * group, then poll(2) should block, similar to the wait(2) family.
+ */
+ if (!task || (task->exit_state && thread_group_empty(task)))
+ poll_flags = POLLIN | POLLRDNORM;
+ rcu_read_unlock();
+
+ return poll_flags;
+}
+
const struct file_operations pidfd_fops = {
.release = pidfd_release,
+ .poll = pidfd_poll,
#ifdef CONFIG_PROC_FS
.show_fdinfo = pidfd_show_fdinfo,
#endif
diff --git a/kernel/pid.c b/kernel/pid.c
index fa704f8..e605398 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -333,6 +333,8 @@ struct pid *alloc_pid(struct pid_namespace *ns)
for (type = 0; type < PIDTYPE_MAX; ++type)
INIT_HLIST_HEAD(&pid->tasks[type]);
+ init_waitqueue_head(&pid->wait_pidfd);
+
upid = pid->numbers + ns->level;
spin_lock_irq(&pidmap_lock);
if (!(ns->nr_hashed & PIDNS_HASH_ADDING))
diff --git a/kernel/signal.c b/kernel/signal.c
index bedca16..053de87a 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1632,6 +1632,14 @@ int send_sigqueue(struct sigqueue *q, struct task_struct *t, int group)
return ret;
}
+static void do_notify_pidfd(struct task_struct *task)
+{
+ struct pid *pid;
+
+ pid = task_pid(task);
+ wake_up_all(&pid->wait_pidfd);
+}
+
/*
* Let a parent know about the death of a child.
* For a stopped/continued status change, use do_notify_parent_cldstop instead.
@@ -1655,6 +1663,9 @@ bool do_notify_parent(struct task_struct *tsk, int sig)
BUG_ON(!tsk->ptrace &&
(tsk->group_leader != tsk || !thread_group_empty(tsk)));
+ /* Wake up all pidfd waiters */
+ do_notify_pidfd(tsk);
+
if (sig != SIGCHLD) {
/*
* This is only possible if parent == real_parent.
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v2 4.9 03/10] proc: Pass file mode to proc_pid_make_inode
2021-01-07 7:52 [PATCH v2 4.9 00/10] fix a race in release_task when flushing the dentry Wen Yang
2021-01-07 7:52 ` [PATCH v2 4.9 01/10] clone: add CLONE_PIDFD Wen Yang
2021-01-07 7:52 ` [PATCH v2 4.9 02/10] pidfd: add polling support Wen Yang
@ 2021-01-07 7:52 ` Wen Yang
2021-01-07 7:52 ` [PATCH v2 4.9 04/10] proc: Better ownership of files for non-dumpable tasks in user namespaces Wen Yang
` (7 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: Wen Yang @ 2021-01-07 7:52 UTC (permalink / raw)
To: Greg Kroah-Hartman, Sasha Levin
Cc: Xunlei Pang, linux-kernel, Andreas Gruenbacher, Paul Moore,
stable, Wen Yang
From: Andreas Gruenbacher <agruenba@redhat.com>
[ Upstream commit db978da8fa1d0819b210c137d31a339149b88875 ]
Pass the file mode of the proc inode to be created to
proc_pid_make_inode. In proc_pid_make_inode, initialize inode->i_mode
before calling security_task_to_inode. This allows selinux to set
isec->sclass right away without introducing "half-initialized" inode
security structs.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
Cc: <stable@vger.kernel.org> # 4.9.x
Signed-off-by: Wen Yang <wenyang@linux.alibaba.com>
---
fs/proc/base.c | 23 +++++++++--------------
fs/proc/fd.c | 6 ++----
fs/proc/internal.h | 2 +-
fs/proc/namespaces.c | 3 +--
security/selinux/hooks.c | 1 +
5 files changed, 14 insertions(+), 21 deletions(-)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index b9e4183..ee2e0ec 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1676,7 +1676,8 @@ static int proc_pid_readlink(struct dentry * dentry, char __user * buffer, int b
/* building an inode */
-struct inode *proc_pid_make_inode(struct super_block * sb, struct task_struct *task)
+struct inode *proc_pid_make_inode(struct super_block * sb,
+ struct task_struct *task, umode_t mode)
{
struct inode * inode;
struct proc_inode *ei;
@@ -1690,6 +1691,7 @@ struct inode *proc_pid_make_inode(struct super_block * sb, struct task_struct *t
/* Common stuff */
ei = PROC_I(inode);
+ inode->i_mode = mode;
inode->i_ino = get_next_ino();
inode->i_mtime = inode->i_atime = inode->i_ctime = current_time(inode);
inode->i_op = &proc_def_inode_operations;
@@ -2041,7 +2043,9 @@ struct map_files_info {
struct proc_inode *ei;
struct inode *inode;
- inode = proc_pid_make_inode(dir->i_sb, task);
+ inode = proc_pid_make_inode(dir->i_sb, task, S_IFLNK |
+ ((mode & FMODE_READ ) ? S_IRUSR : 0) |
+ ((mode & FMODE_WRITE) ? S_IWUSR : 0));
if (!inode)
return -ENOENT;
@@ -2050,12 +2054,6 @@ struct map_files_info {
inode->i_op = &proc_map_files_link_inode_operations;
inode->i_size = 64;
- inode->i_mode = S_IFLNK;
-
- if (mode & FMODE_READ)
- inode->i_mode |= S_IRUSR;
- if (mode & FMODE_WRITE)
- inode->i_mode |= S_IWUSR;
d_set_d_op(dentry, &tid_map_files_dentry_operations);
d_add(dentry, inode);
@@ -2409,12 +2407,11 @@ static int proc_pident_instantiate(struct inode *dir,
struct inode *inode;
struct proc_inode *ei;
- inode = proc_pid_make_inode(dir->i_sb, task);
+ inode = proc_pid_make_inode(dir->i_sb, task, p->mode);
if (!inode)
goto out;
ei = PROC_I(inode);
- inode->i_mode = p->mode;
if (S_ISDIR(inode->i_mode))
set_nlink(inode, 2); /* Use getattr to fix if necessary */
if (p->iop)
@@ -3096,11 +3093,10 @@ static int proc_pid_instantiate(struct inode *dir,
{
struct inode *inode;
- inode = proc_pid_make_inode(dir->i_sb, task);
+ inode = proc_pid_make_inode(dir->i_sb, task, S_IFDIR | S_IRUGO | S_IXUGO);
if (!inode)
goto out;
- inode->i_mode = S_IFDIR|S_IRUGO|S_IXUGO;
inode->i_op = &proc_tgid_base_inode_operations;
inode->i_fop = &proc_tgid_base_operations;
inode->i_flags|=S_IMMUTABLE;
@@ -3391,11 +3387,10 @@ static int proc_task_instantiate(struct inode *dir,
struct dentry *dentry, struct task_struct *task, const void *ptr)
{
struct inode *inode;
- inode = proc_pid_make_inode(dir->i_sb, task);
+ inode = proc_pid_make_inode(dir->i_sb, task, S_IFDIR | S_IRUGO | S_IXUGO);
if (!inode)
goto out;
- inode->i_mode = S_IFDIR|S_IRUGO|S_IXUGO;
inode->i_op = &proc_tid_base_inode_operations;
inode->i_fop = &proc_tid_base_operations;
inode->i_flags|=S_IMMUTABLE;
diff --git a/fs/proc/fd.c b/fs/proc/fd.c
index d21dafe..4274f83 100644
--- a/fs/proc/fd.c
+++ b/fs/proc/fd.c
@@ -183,14 +183,13 @@ static int proc_fd_link(struct dentry *dentry, struct path *path)
struct proc_inode *ei;
struct inode *inode;
- inode = proc_pid_make_inode(dir->i_sb, task);
+ inode = proc_pid_make_inode(dir->i_sb, task, S_IFLNK);
if (!inode)
goto out;
ei = PROC_I(inode);
ei->fd = fd;
- inode->i_mode = S_IFLNK;
inode->i_op = &proc_pid_link_inode_operations;
inode->i_size = 64;
@@ -322,14 +321,13 @@ int proc_fd_permission(struct inode *inode, int mask)
struct proc_inode *ei;
struct inode *inode;
- inode = proc_pid_make_inode(dir->i_sb, task);
+ inode = proc_pid_make_inode(dir->i_sb, task, S_IFREG | S_IRUSR);
if (!inode)
goto out;
ei = PROC_I(inode);
ei->fd = fd;
- inode->i_mode = S_IFREG | S_IRUSR;
inode->i_fop = &proc_fdinfo_file_operations;
d_set_d_op(dentry, &tid_fd_dentry_operations);
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index c0bdece..5bc057b 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -163,7 +163,7 @@ extern int proc_pid_statm(struct seq_file *, struct pid_namespace *,
extern const struct dentry_operations pid_dentry_operations;
extern int pid_getattr(struct vfsmount *, struct dentry *, struct kstat *);
extern int proc_setattr(struct dentry *, struct iattr *);
-extern struct inode *proc_pid_make_inode(struct super_block *, struct task_struct *);
+extern struct inode *proc_pid_make_inode(struct super_block *, struct task_struct *, umode_t);
extern int pid_revalidate(struct dentry *, unsigned int);
extern int pid_delete_dentry(const struct dentry *);
extern int proc_pid_readdir(struct file *, struct dir_context *);
diff --git a/fs/proc/namespaces.c b/fs/proc/namespaces.c
index 51b8b0a..766f0c6 100644
--- a/fs/proc/namespaces.c
+++ b/fs/proc/namespaces.c
@@ -92,12 +92,11 @@ static int proc_ns_instantiate(struct inode *dir,
struct inode *inode;
struct proc_inode *ei;
- inode = proc_pid_make_inode(dir->i_sb, task);
+ inode = proc_pid_make_inode(dir->i_sb, task, S_IFLNK | S_IRWXUGO);
if (!inode)
goto out;
ei = PROC_I(inode);
- inode->i_mode = S_IFLNK|S_IRWXUGO;
inode->i_op = &proc_ns_link_inode_operations;
ei->ns_ops = ns_ops;
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index a2b63a6..98b5f40 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3967,6 +3967,7 @@ static void selinux_task_to_inode(struct task_struct *p,
struct inode_security_struct *isec = inode->i_security;
u32 sid = task_sid(p);
+ isec->sclass = inode_mode_to_security_class(inode->i_mode);
isec->sid = sid;
isec->initialized = LABEL_INITIALIZED;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v2 4.9 04/10] proc: Better ownership of files for non-dumpable tasks in user namespaces
2021-01-07 7:52 [PATCH v2 4.9 00/10] fix a race in release_task when flushing the dentry Wen Yang
` (2 preceding siblings ...)
2021-01-07 7:52 ` [PATCH v2 4.9 03/10] proc: Pass file mode to proc_pid_make_inode Wen Yang
@ 2021-01-07 7:52 ` Wen Yang
2021-01-07 7:52 ` [PATCH v2 4.9 05/10] proc: use %u for pid printing and slightly less stack Wen Yang
` (6 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: Wen Yang @ 2021-01-07 7:52 UTC (permalink / raw)
To: Greg Kroah-Hartman, Sasha Levin
Cc: Xunlei Pang, linux-kernel, Eric W. Biederman, stable, Wen Yang
From: "Eric W. Biederman" <ebiederm@xmission.com>
[ Upstream commit 68eb94f16227336a5773b83ecfa8290f1d6b78ce ]
Instead of making the files owned by the GLOBAL_ROOT_USER. Make
non-dumpable files whose mm has always lived in a user namespace owned
by the user namespace root. This allows the container root to have
things work as expected in a container.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: <stable@vger.kernel.org> # 4.9.x
Signed-off-by: Wen Yang <wenyang@linux.alibaba.com>
---
fs/proc/base.c | 102 ++++++++++++++++++++++++++++++-----------------------
fs/proc/fd.c | 12 +------
fs/proc/internal.h | 16 ++-------
3 files changed, 61 insertions(+), 69 deletions(-)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index ee2e0ec..5bfdb61 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1676,12 +1676,63 @@ static int proc_pid_readlink(struct dentry * dentry, char __user * buffer, int b
/* building an inode */
+void task_dump_owner(struct task_struct *task, mode_t mode,
+ kuid_t *ruid, kgid_t *rgid)
+{
+ /* Depending on the state of dumpable compute who should own a
+ * proc file for a task.
+ */
+ const struct cred *cred;
+ kuid_t uid;
+ kgid_t gid;
+
+ /* Default to the tasks effective ownership */
+ rcu_read_lock();
+ cred = __task_cred(task);
+ uid = cred->euid;
+ gid = cred->egid;
+ rcu_read_unlock();
+
+ /*
+ * Before the /proc/pid/status file was created the only way to read
+ * the effective uid of a /process was to stat /proc/pid. Reading
+ * /proc/pid/status is slow enough that procps and other packages
+ * kept stating /proc/pid. To keep the rules in /proc simple I have
+ * made this apply to all per process world readable and executable
+ * directories.
+ */
+ if (mode != (S_IFDIR|S_IRUGO|S_IXUGO)) {
+ struct mm_struct *mm;
+ task_lock(task);
+ mm = task->mm;
+ /* Make non-dumpable tasks owned by some root */
+ if (mm) {
+ if (get_dumpable(mm) != SUID_DUMP_USER) {
+ struct user_namespace *user_ns = mm->user_ns;
+
+ uid = make_kuid(user_ns, 0);
+ if (!uid_valid(uid))
+ uid = GLOBAL_ROOT_UID;
+
+ gid = make_kgid(user_ns, 0);
+ if (!gid_valid(gid))
+ gid = GLOBAL_ROOT_GID;
+ }
+ } else {
+ uid = GLOBAL_ROOT_UID;
+ gid = GLOBAL_ROOT_GID;
+ }
+ task_unlock(task);
+ }
+ *ruid = uid;
+ *rgid = gid;
+}
+
struct inode *proc_pid_make_inode(struct super_block * sb,
struct task_struct *task, umode_t mode)
{
struct inode * inode;
struct proc_inode *ei;
- const struct cred *cred;
/* We need a new inode */
@@ -1703,13 +1754,7 @@ struct inode *proc_pid_make_inode(struct super_block * sb,
if (!ei->pid)
goto out_unlock;
- if (task_dumpable(task)) {
- rcu_read_lock();
- cred = __task_cred(task);
- inode->i_uid = cred->euid;
- inode->i_gid = cred->egid;
- rcu_read_unlock();
- }
+ task_dump_owner(task, 0, &inode->i_uid, &inode->i_gid);
security_task_to_inode(task, inode);
out:
@@ -1724,7 +1769,6 @@ int pid_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat)
{
struct inode *inode = d_inode(dentry);
struct task_struct *task;
- const struct cred *cred;
struct pid_namespace *pid = dentry->d_sb->s_fs_info;
generic_fillattr(inode, stat);
@@ -1742,12 +1786,7 @@ int pid_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat)
*/
return -ENOENT;
}
- if ((inode->i_mode == (S_IFDIR|S_IRUGO|S_IXUGO)) ||
- task_dumpable(task)) {
- cred = __task_cred(task);
- stat->uid = cred->euid;
- stat->gid = cred->egid;
- }
+ task_dump_owner(task, inode->i_mode, &stat->uid, &stat->gid);
}
rcu_read_unlock();
return 0;
@@ -1763,18 +1802,11 @@ int pid_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat)
* Rewrite the inode's ownerships here because the owning task may have
* performed a setuid(), etc.
*
- * Before the /proc/pid/status file was created the only way to read
- * the effective uid of a /process was to stat /proc/pid. Reading
- * /proc/pid/status is slow enough that procps and other packages
- * kept stating /proc/pid. To keep the rules in /proc simple I have
- * made this apply to all per process world readable and executable
- * directories.
*/
int pid_revalidate(struct dentry *dentry, unsigned int flags)
{
struct inode *inode;
struct task_struct *task;
- const struct cred *cred;
if (flags & LOOKUP_RCU)
return -ECHILD;
@@ -1783,17 +1815,8 @@ int pid_revalidate(struct dentry *dentry, unsigned int flags)
task = get_proc_task(inode);
if (task) {
- if ((inode->i_mode == (S_IFDIR|S_IRUGO|S_IXUGO)) ||
- task_dumpable(task)) {
- rcu_read_lock();
- cred = __task_cred(task);
- inode->i_uid = cred->euid;
- inode->i_gid = cred->egid;
- rcu_read_unlock();
- } else {
- inode->i_uid = GLOBAL_ROOT_UID;
- inode->i_gid = GLOBAL_ROOT_GID;
- }
+ task_dump_owner(task, inode->i_mode, &inode->i_uid, &inode->i_gid);
+
inode->i_mode &= ~(S_ISUID | S_ISGID);
security_task_to_inode(task, inode);
put_task_struct(task);
@@ -1915,7 +1938,6 @@ static int map_files_d_revalidate(struct dentry *dentry, unsigned int flags)
bool exact_vma_exists = false;
struct mm_struct *mm = NULL;
struct task_struct *task;
- const struct cred *cred;
struct inode *inode;
int status = 0;
@@ -1940,16 +1962,8 @@ static int map_files_d_revalidate(struct dentry *dentry, unsigned int flags)
mmput(mm);
if (exact_vma_exists) {
- if (task_dumpable(task)) {
- rcu_read_lock();
- cred = __task_cred(task);
- inode->i_uid = cred->euid;
- inode->i_gid = cred->egid;
- rcu_read_unlock();
- } else {
- inode->i_uid = GLOBAL_ROOT_UID;
- inode->i_gid = GLOBAL_ROOT_GID;
- }
+ task_dump_owner(task, 0, &inode->i_uid, &inode->i_gid);
+
security_task_to_inode(task, inode);
status = 1;
}
diff --git a/fs/proc/fd.c b/fs/proc/fd.c
index 4274f83..00ce153 100644
--- a/fs/proc/fd.c
+++ b/fs/proc/fd.c
@@ -84,7 +84,6 @@ static int tid_fd_revalidate(struct dentry *dentry, unsigned int flags)
{
struct files_struct *files;
struct task_struct *task;
- const struct cred *cred;
struct inode *inode;
unsigned int fd;
@@ -108,16 +107,7 @@ static int tid_fd_revalidate(struct dentry *dentry, unsigned int flags)
rcu_read_unlock();
put_files_struct(files);
- if (task_dumpable(task)) {
- rcu_read_lock();
- cred = __task_cred(task);
- inode->i_uid = cred->euid;
- inode->i_gid = cred->egid;
- rcu_read_unlock();
- } else {
- inode->i_uid = GLOBAL_ROOT_UID;
- inode->i_gid = GLOBAL_ROOT_GID;
- }
+ task_dump_owner(task, 0, &inode->i_uid, &inode->i_gid);
if (S_ISLNK(inode->i_mode)) {
unsigned i_mode = S_IFLNK;
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index 5bc057b..103435f 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -98,20 +98,8 @@ static inline struct task_struct *get_proc_task(struct inode *inode)
return get_pid_task(proc_pid(inode), PIDTYPE_PID);
}
-static inline int task_dumpable(struct task_struct *task)
-{
- int dumpable = 0;
- struct mm_struct *mm;
-
- task_lock(task);
- mm = task->mm;
- if (mm)
- dumpable = get_dumpable(mm);
- task_unlock(task);
- if (dumpable == SUID_DUMP_USER)
- return 1;
- return 0;
-}
+void task_dump_owner(struct task_struct *task, mode_t mode,
+ kuid_t *ruid, kgid_t *rgid);
static inline unsigned name_to_int(const struct qstr *qstr)
{
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v2 4.9 05/10] proc: use %u for pid printing and slightly less stack
2021-01-07 7:52 [PATCH v2 4.9 00/10] fix a race in release_task when flushing the dentry Wen Yang
` (3 preceding siblings ...)
2021-01-07 7:52 ` [PATCH v2 4.9 04/10] proc: Better ownership of files for non-dumpable tasks in user namespaces Wen Yang
@ 2021-01-07 7:52 ` Wen Yang
2021-01-07 7:52 ` [PATCH v2 4.9 06/10] proc: Rename in proc_inode rename sysctl_inodes sibling_inodes Wen Yang
` (5 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: Wen Yang @ 2021-01-07 7:52 UTC (permalink / raw)
To: Greg Kroah-Hartman, Sasha Levin
Cc: Xunlei Pang, linux-kernel, Alexey Dobriyan, Alexander Viro,
Andrew Morton, Linus Torvalds, stable, Wen Yang
From: Alexey Dobriyan <adobriyan@gmail.com>
[ Upstream commit e3912ac37e07a13c70675cd75020694de4841c74 ]
PROC_NUMBUF is 13 which is enough for "negative int + \n + \0".
However PIDs and TGIDs are never negative and newline is not a concern,
so use just 10 per integer.
Link: http://lkml.kernel.org/r/20171120203005.GA27743@avx2
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Alexander Viro <viro@ftp.linux.org.uk>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: <stable@vger.kernel.org> # 4.9.x
Signed-off-by: Wen Yang <wenyang@linux.alibaba.com>
---
fs/proc/base.c | 16 ++++++++--------
fs/proc/fd.c | 2 +-
fs/proc/self.c | 6 +++---
fs/proc/thread_self.c | 5 ++---
4 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 5bfdb61..3502a40 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -3018,11 +3018,11 @@ static struct dentry *proc_tgid_base_lookup(struct inode *dir, struct dentry *de
static void proc_flush_task_mnt(struct vfsmount *mnt, pid_t pid, pid_t tgid)
{
struct dentry *dentry, *leader, *dir;
- char buf[PROC_NUMBUF];
+ char buf[10 + 1];
struct qstr name;
name.name = buf;
- name.len = snprintf(buf, sizeof(buf), "%d", pid);
+ name.len = snprintf(buf, sizeof(buf), "%u", pid);
/* no ->d_hash() rejects on procfs */
dentry = d_hash_and_lookup(mnt->mnt_root, &name);
if (dentry) {
@@ -3034,7 +3034,7 @@ static void proc_flush_task_mnt(struct vfsmount *mnt, pid_t pid, pid_t tgid)
return;
name.name = buf;
- name.len = snprintf(buf, sizeof(buf), "%d", tgid);
+ name.len = snprintf(buf, sizeof(buf), "%u", tgid);
leader = d_hash_and_lookup(mnt->mnt_root, &name);
if (!leader)
goto out;
@@ -3046,7 +3046,7 @@ static void proc_flush_task_mnt(struct vfsmount *mnt, pid_t pid, pid_t tgid)
goto out_put_leader;
name.name = buf;
- name.len = snprintf(buf, sizeof(buf), "%d", pid);
+ name.len = snprintf(buf, sizeof(buf), "%u", pid);
dentry = d_hash_and_lookup(dir, &name);
if (dentry) {
d_invalidate(dentry);
@@ -3226,14 +3226,14 @@ int proc_pid_readdir(struct file *file, struct dir_context *ctx)
for (iter = next_tgid(ns, iter);
iter.task;
iter.tgid += 1, iter = next_tgid(ns, iter)) {
- char name[PROC_NUMBUF];
+ char name[10 + 1];
int len;
cond_resched();
if (!has_pid_permissions(ns, iter.task, 2))
continue;
- len = snprintf(name, sizeof(name), "%d", iter.tgid);
+ len = snprintf(name, sizeof(name), "%u", iter.tgid);
ctx->pos = iter.tgid + TGID_OFFSET;
if (!proc_fill_cache(file, ctx, name, len,
proc_pid_instantiate, iter.task, NULL)) {
@@ -3557,10 +3557,10 @@ static int proc_task_readdir(struct file *file, struct dir_context *ctx)
for (task = first_tid(proc_pid(inode), tid, ctx->pos - 2, ns);
task;
task = next_tid(task), ctx->pos++) {
- char name[PROC_NUMBUF];
+ char name[10 + 1];
int len;
tid = task_pid_nr_ns(task, ns);
- len = snprintf(name, sizeof(name), "%d", tid);
+ len = snprintf(name, sizeof(name), "%u", tid);
if (!proc_fill_cache(file, ctx, name, len,
proc_task_instantiate, task, NULL)) {
/* returning this tgid failed, save it as the first
diff --git a/fs/proc/fd.c b/fs/proc/fd.c
index 00ce153..390c2fe 100644
--- a/fs/proc/fd.c
+++ b/fs/proc/fd.c
@@ -235,7 +235,7 @@ static int proc_readfd_common(struct file *file, struct dir_context *ctx,
for (fd = ctx->pos - 2;
fd < files_fdtable(files)->max_fds;
fd++, ctx->pos++) {
- char name[PROC_NUMBUF];
+ char name[10 + 1];
int len;
if (!fcheck_files(files, fd))
diff --git a/fs/proc/self.c b/fs/proc/self.c
index f6e2e3f..dd06755 100644
--- a/fs/proc/self.c
+++ b/fs/proc/self.c
@@ -35,11 +35,11 @@ static const char *proc_self_get_link(struct dentry *dentry,
if (!tgid)
return ERR_PTR(-ENOENT);
- /* 11 for max length of signed int in decimal + NULL term */
- name = kmalloc(12, dentry ? GFP_KERNEL : GFP_ATOMIC);
+ /* max length of unsigned int in decimal + NULL term */
+ name = kmalloc(10 + 1, dentry ? GFP_KERNEL : GFP_ATOMIC);
if (unlikely(!name))
return dentry ? ERR_PTR(-ENOMEM) : ERR_PTR(-ECHILD);
- sprintf(name, "%d", tgid);
+ sprintf(name, "%u", tgid);
set_delayed_call(done, kfree_link, name);
return name;
}
diff --git a/fs/proc/thread_self.c b/fs/proc/thread_self.c
index 02d1db8..44e0921 100644
--- a/fs/proc/thread_self.c
+++ b/fs/proc/thread_self.c
@@ -30,11 +30,10 @@ static const char *proc_thread_self_get_link(struct dentry *dentry,
if (!pid)
return ERR_PTR(-ENOENT);
- name = kmalloc(PROC_NUMBUF + 6 + PROC_NUMBUF,
- dentry ? GFP_KERNEL : GFP_ATOMIC);
+ name = kmalloc(10 + 6 + 10 + 1, dentry ? GFP_KERNEL : GFP_ATOMIC);
if (unlikely(!name))
return dentry ? ERR_PTR(-ENOMEM) : ERR_PTR(-ECHILD);
- sprintf(name, "%d/task/%d", tgid, pid);
+ sprintf(name, "%u/task/%u", tgid, pid);
set_delayed_call(done, kfree_link, name);
return name;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v2 4.9 06/10] proc: Rename in proc_inode rename sysctl_inodes sibling_inodes
2021-01-07 7:52 [PATCH v2 4.9 00/10] fix a race in release_task when flushing the dentry Wen Yang
` (4 preceding siblings ...)
2021-01-07 7:52 ` [PATCH v2 4.9 05/10] proc: use %u for pid printing and slightly less stack Wen Yang
@ 2021-01-07 7:52 ` Wen Yang
2021-01-07 7:52 ` [PATCH v2 4.9 07/10] proc: Generalize proc_sys_prune_dcache into proc_prune_siblings_dcache Wen Yang
` (4 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: Wen Yang @ 2021-01-07 7:52 UTC (permalink / raw)
To: Greg Kroah-Hartman, Sasha Levin
Cc: Xunlei Pang, linux-kernel, Eric W. Biederman, stable, Wen Yang
From: "Eric W. Biederman" <ebiederm@xmission.com>
[ Upstream commit 0afa5ca82212247456f9de1468b595a111fee633 ]
I about to need and use the same functionality for pid based
inodes and there is no point in adding a second field when
this field is already here and serving the same purporse.
Just give the field a generic name so it is clear that
it is no longer sysctl specific.
Also for good measure initialize sibling_inodes when
proc_inode is initialized.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Cc: <stable@vger.kernel.org> # 4.9.x
Signed-off-by: Wen Yang <wenyang@linux.alibaba.com>
---
fs/proc/inode.c | 1 +
fs/proc/internal.h | 2 +-
fs/proc/proc_sysctl.c | 8 ++++----
3 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index a289349..14d9c1d 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -67,6 +67,7 @@ static struct inode *proc_alloc_inode(struct super_block *sb)
ei->pde = NULL;
ei->sysctl = NULL;
ei->sysctl_entry = NULL;
+ INIT_HLIST_NODE(&ei->sibling_inodes);
ei->ns_ops = NULL;
inode = &ei->vfs_inode;
return inode;
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index 103435f..409b5c5 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -65,7 +65,7 @@ struct proc_inode {
struct proc_dir_entry *pde;
struct ctl_table_header *sysctl;
struct ctl_table *sysctl_entry;
- struct hlist_node sysctl_inodes;
+ struct hlist_node sibling_inodes;
const struct proc_ns_operations *ns_ops;
struct inode vfs_inode;
};
diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 191573a..671490e 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -272,9 +272,9 @@ static void proc_sys_prune_dcache(struct ctl_table_header *head)
node = hlist_first_rcu(&head->inodes);
if (!node)
break;
- ei = hlist_entry(node, struct proc_inode, sysctl_inodes);
+ ei = hlist_entry(node, struct proc_inode, sibling_inodes);
spin_lock(&sysctl_lock);
- hlist_del_init_rcu(&ei->sysctl_inodes);
+ hlist_del_init_rcu(&ei->sibling_inodes);
spin_unlock(&sysctl_lock);
inode = &ei->vfs_inode;
@@ -480,7 +480,7 @@ static struct inode *proc_sys_make_inode(struct super_block *sb,
}
ei->sysctl = head;
ei->sysctl_entry = table;
- hlist_add_head_rcu(&ei->sysctl_inodes, &head->inodes);
+ hlist_add_head_rcu(&ei->sibling_inodes, &head->inodes);
head->count++;
spin_unlock(&sysctl_lock);
@@ -511,7 +511,7 @@ static struct inode *proc_sys_make_inode(struct super_block *sb,
void proc_sys_evict_inode(struct inode *inode, struct ctl_table_header *head)
{
spin_lock(&sysctl_lock);
- hlist_del_init_rcu(&PROC_I(inode)->sysctl_inodes);
+ hlist_del_init_rcu(&PROC_I(inode)->sibling_inodes);
if (!--head->count)
kfree_rcu(head, rcu);
spin_unlock(&sysctl_lock);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v2 4.9 07/10] proc: Generalize proc_sys_prune_dcache into proc_prune_siblings_dcache
2021-01-07 7:52 [PATCH v2 4.9 00/10] fix a race in release_task when flushing the dentry Wen Yang
` (5 preceding siblings ...)
2021-01-07 7:52 ` [PATCH v2 4.9 06/10] proc: Rename in proc_inode rename sysctl_inodes sibling_inodes Wen Yang
@ 2021-01-07 7:52 ` Wen Yang
2021-01-07 7:52 ` [PATCH v2 4.9 08/10] proc: Clear the pieces of proc_inode that proc_evict_inode cares about Wen Yang
` (3 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: Wen Yang @ 2021-01-07 7:52 UTC (permalink / raw)
To: Greg Kroah-Hartman, Sasha Levin
Cc: Xunlei Pang, linux-kernel, Eric W. Biederman, stable, Wen Yang
From: "Eric W. Biederman" <ebiederm@xmission.com>
[ Upstream commit 26dbc60f385ff9cff475ea2a3bad02e80fd6fa43 ]
This prepares the way for allowing the pid part of proc to use this
dcache pruning code as well.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Cc: <stable@vger.kernel.org> # 4.9.x
(proc: fix up cherry-pick conflicts for 26dbc60f385f)
Signed-off-by: Wen Yang <wenyang@linux.alibaba.com>
---
fs/proc/inode.c | 38 ++++++++++++++++++++++++++++++++++++++
fs/proc/internal.h | 1 +
fs/proc/proc_sysctl.c | 35 +----------------------------------
3 files changed, 40 insertions(+), 34 deletions(-)
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index 14d9c1d..920c761 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -101,6 +101,44 @@ void __init proc_init_inodecache(void)
init_once);
}
+void proc_prune_siblings_dcache(struct hlist_head *inodes, spinlock_t *lock)
+{
+ struct inode *inode;
+ struct proc_inode *ei;
+ struct hlist_node *node;
+ struct super_block *sb;
+
+ rcu_read_lock();
+ for (;;) {
+ node = hlist_first_rcu(inodes);
+ if (!node)
+ break;
+ ei = hlist_entry(node, struct proc_inode, sibling_inodes);
+ spin_lock(lock);
+ hlist_del_init_rcu(&ei->sibling_inodes);
+ spin_unlock(lock);
+
+ inode = &ei->vfs_inode;
+ sb = inode->i_sb;
+ if (!atomic_inc_not_zero(&sb->s_active))
+ continue;
+ inode = igrab(inode);
+ rcu_read_unlock();
+ if (unlikely(!inode)) {
+ deactivate_super(sb);
+ rcu_read_lock();
+ continue;
+ }
+
+ d_prune_aliases(inode);
+ iput(inode);
+ deactivate_super(sb);
+
+ rcu_read_lock();
+ }
+ rcu_read_unlock();
+}
+
static int proc_show_options(struct seq_file *seq, struct dentry *root)
{
struct super_block *sb = root->d_sb;
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index 409b5c5..9bc44a1 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -200,6 +200,7 @@ struct pde_opener {
extern const struct inode_operations proc_pid_link_inode_operations;
extern void proc_init_inodecache(void);
+void proc_prune_siblings_dcache(struct hlist_head *inodes, spinlock_t *lock);
extern struct inode *proc_get_inode(struct super_block *, struct proc_dir_entry *);
extern int proc_fill_super(struct super_block *, void *data, int flags);
extern void proc_entry_rundown(struct proc_dir_entry *);
diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 671490e..f19063b 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -262,40 +262,7 @@ static void unuse_table(struct ctl_table_header *p)
static void proc_sys_prune_dcache(struct ctl_table_header *head)
{
- struct inode *inode;
- struct proc_inode *ei;
- struct hlist_node *node;
- struct super_block *sb;
-
- rcu_read_lock();
- for (;;) {
- node = hlist_first_rcu(&head->inodes);
- if (!node)
- break;
- ei = hlist_entry(node, struct proc_inode, sibling_inodes);
- spin_lock(&sysctl_lock);
- hlist_del_init_rcu(&ei->sibling_inodes);
- spin_unlock(&sysctl_lock);
-
- inode = &ei->vfs_inode;
- sb = inode->i_sb;
- if (!atomic_inc_not_zero(&sb->s_active))
- continue;
- inode = igrab(inode);
- rcu_read_unlock();
- if (unlikely(!inode)) {
- deactivate_super(sb);
- rcu_read_lock();
- continue;
- }
-
- d_prune_aliases(inode);
- iput(inode);
- deactivate_super(sb);
-
- rcu_read_lock();
- }
- rcu_read_unlock();
+ proc_prune_siblings_dcache(&head->inodes, &sysctl_lock);
}
/* called under sysctl_lock, will reacquire if has to wait */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v2 4.9 08/10] proc: Clear the pieces of proc_inode that proc_evict_inode cares about
2021-01-07 7:52 [PATCH v2 4.9 00/10] fix a race in release_task when flushing the dentry Wen Yang
` (6 preceding siblings ...)
2021-01-07 7:52 ` [PATCH v2 4.9 07/10] proc: Generalize proc_sys_prune_dcache into proc_prune_siblings_dcache Wen Yang
@ 2021-01-07 7:52 ` Wen Yang
2021-01-07 7:52 ` [PATCH v2 4.9 09/10] proc: Use d_invalidate in proc_prune_siblings_dcache Wen Yang
` (2 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: Wen Yang @ 2021-01-07 7:52 UTC (permalink / raw)
To: Greg Kroah-Hartman, Sasha Levin
Cc: Xunlei Pang, linux-kernel, Eric W. Biederman, stable, Wen Yang
From: "Eric W. Biederman" <ebiederm@xmission.com>
[ Upstream commit 71448011ea2a1cd36d8f5cbdab0ed716c454d565 ]
This just keeps everything tidier, and allows for using flags like
SLAB_TYPESAFE_BY_RCU where slabs are not always cleared before reuse.
I don't see reuse without reinitializing happening with the proc_inode
but I had a false alarm while reworking flushing of proc dentries and
indoes when a process dies that caused me to tidy this up.
The code is a little easier to follow and reason about this
way so I figured the changes might as well be kept.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: <stable@vger.kernel.org> # 4.9.x
Signed-off-by: Wen Yang <wenyang@linux.alibaba.com>
---
fs/proc/inode.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index 920c761..739fb9c 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -32,21 +32,27 @@ static void proc_evict_inode(struct inode *inode)
{
struct proc_dir_entry *de;
struct ctl_table_header *head;
+ struct proc_inode *ei = PROC_I(inode);
truncate_inode_pages_final(&inode->i_data);
clear_inode(inode);
/* Stop tracking associated processes */
- put_pid(PROC_I(inode)->pid);
+ if (ei->pid) {
+ put_pid(ei->pid);
+ ei->pid = NULL;
+ }
/* Let go of any associated proc directory entry */
- de = PDE(inode);
- if (de)
+ de = ei->pde;
+ if (de) {
pde_put(de);
+ ei->pde = NULL;
+ }
- head = PROC_I(inode)->sysctl;
+ head = ei->sysctl;
if (head) {
- RCU_INIT_POINTER(PROC_I(inode)->sysctl, NULL);
+ RCU_INIT_POINTER(ei->sysctl, NULL);
proc_sys_evict_inode(inode, head);
}
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v2 4.9 09/10] proc: Use d_invalidate in proc_prune_siblings_dcache
2021-01-07 7:52 [PATCH v2 4.9 00/10] fix a race in release_task when flushing the dentry Wen Yang
` (7 preceding siblings ...)
2021-01-07 7:52 ` [PATCH v2 4.9 08/10] proc: Clear the pieces of proc_inode that proc_evict_inode cares about Wen Yang
@ 2021-01-07 7:52 ` Wen Yang
2021-01-07 7:52 ` [PATCH v2 4.9 10/10] proc: Use a list of inodes to flush from proc Wen Yang
2021-01-07 12:17 ` [PATCH v2 4.9 00/10] fix a race in release_task when flushing the dentry Greg Kroah-Hartman
10 siblings, 0 replies; 17+ messages in thread
From: Wen Yang @ 2021-01-07 7:52 UTC (permalink / raw)
To: Greg Kroah-Hartman, Sasha Levin
Cc: Xunlei Pang, linux-kernel, Eric W. Biederman, stable, Wen Yang
From: "Eric W. Biederman" <ebiederm@xmission.com>
[ Upstream commit f90f3cafe8d56d593fc509a4185da1d5800efea4 ]
The function d_prune_aliases has the problem that it will only prune
aliases thare are completely unused. It will not remove aliases for
the dcache or even think of removing mounts from the dcache. For that
behavior d_invalidate is needed.
To use d_invalidate replace d_prune_aliases with d_find_alias followed
by d_invalidate and dput.
For completeness the directory and the non-directory cases are
separated because in theory (although not in currently in practice for
proc) directories can only ever have a single dentry while
non-directories can have hardlinks and thus multiple dentries.
As part of this separation use d_find_any_alias for directories
to spare d_find_alias the extra work of doing that.
Plus the differences between d_find_any_alias and d_find_alias makes
it clear why the directory and non-directory code and not share code.
To make it clear these routines now invalidate dentries rename
proc_prune_siblings_dache to proc_invalidate_siblings_dcache, and rename
proc_sys_prune_dcache proc_sys_invalidate_dcache.
V2: Split the directory and non-directory cases. To make this
code robust to future changes in proc.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: <stable@vger.kernel.org> # 4.9.x
(proc: fix up cherry-pick conflicts for f90f3cafe8d5)
Signed-off-by: Wen Yang <wenyang@linux.alibaba.com>
---
fs/proc/inode.c | 16 ++++++++++++++--
fs/proc/internal.h | 2 +-
fs/proc/proc_sysctl.c | 8 ++++----
3 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index 739fb9c..2af9f4f 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -107,7 +107,7 @@ void __init proc_init_inodecache(void)
init_once);
}
-void proc_prune_siblings_dcache(struct hlist_head *inodes, spinlock_t *lock)
+void proc_invalidate_siblings_dcache(struct hlist_head *inodes, spinlock_t *lock)
{
struct inode *inode;
struct proc_inode *ei;
@@ -136,7 +136,19 @@ void proc_prune_siblings_dcache(struct hlist_head *inodes, spinlock_t *lock)
continue;
}
- d_prune_aliases(inode);
+ if (S_ISDIR(inode->i_mode)) {
+ struct dentry *dir = d_find_any_alias(inode);
+ if (dir) {
+ d_invalidate(dir);
+ dput(dir);
+ }
+ } else {
+ struct dentry *dentry;
+ while ((dentry = d_find_alias(inode))) {
+ d_invalidate(dentry);
+ dput(dentry);
+ }
+ }
iput(inode);
deactivate_super(sb);
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index 9bc44a1..6a1d679 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -200,7 +200,7 @@ struct pde_opener {
extern const struct inode_operations proc_pid_link_inode_operations;
extern void proc_init_inodecache(void);
-void proc_prune_siblings_dcache(struct hlist_head *inodes, spinlock_t *lock);
+void proc_invalidate_siblings_dcache(struct hlist_head *inodes, spinlock_t *lock);
extern struct inode *proc_get_inode(struct super_block *, struct proc_dir_entry *);
extern int proc_fill_super(struct super_block *, void *data, int flags);
extern void proc_entry_rundown(struct proc_dir_entry *);
diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index f19063b..b6668a5 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -260,9 +260,9 @@ static void unuse_table(struct ctl_table_header *p)
complete(p->unregistering);
}
-static void proc_sys_prune_dcache(struct ctl_table_header *head)
+static void proc_sys_invalidate_dcache(struct ctl_table_header *head)
{
- proc_prune_siblings_dcache(&head->inodes, &sysctl_lock);
+ proc_invalidate_siblings_dcache(&head->inodes, &sysctl_lock);
}
/* called under sysctl_lock, will reacquire if has to wait */
@@ -284,10 +284,10 @@ static void start_unregistering(struct ctl_table_header *p)
spin_unlock(&sysctl_lock);
}
/*
- * Prune dentries for unregistered sysctls: namespaced sysctls
+ * Invalidate dentries for unregistered sysctls: namespaced sysctls
* can have duplicate names and contaminate dcache very badly.
*/
- proc_sys_prune_dcache(p);
+ proc_sys_invalidate_dcache(p);
/*
* do not remove from the list until nobody holds it; walking the
* list in do_sysctl() relies on that.
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v2 4.9 10/10] proc: Use a list of inodes to flush from proc
2021-01-07 7:52 [PATCH v2 4.9 00/10] fix a race in release_task when flushing the dentry Wen Yang
` (8 preceding siblings ...)
2021-01-07 7:52 ` [PATCH v2 4.9 09/10] proc: Use d_invalidate in proc_prune_siblings_dcache Wen Yang
@ 2021-01-07 7:52 ` Wen Yang
2021-01-07 12:17 ` [PATCH v2 4.9 00/10] fix a race in release_task when flushing the dentry Greg Kroah-Hartman
10 siblings, 0 replies; 17+ messages in thread
From: Wen Yang @ 2021-01-07 7:52 UTC (permalink / raw)
To: Greg Kroah-Hartman, Sasha Levin
Cc: Xunlei Pang, linux-kernel, Eric W. Biederman, stable, Wen Yang
From: "Eric W. Biederman" <ebiederm@xmission.com>
[ Upstream commit 7bc3e6e55acf065500a24621f3b313e7e5998acf ]
Rework the flushing of proc to use a list of directory inodes that
need to be flushed.
The list is kept on struct pid not on struct task_struct, as there is
a fixed connection between proc inodes and pids but at least for the
case of de_thread the pid of a task_struct changes.
This removes the dependency on proc_mnt which allows for different
mounts of proc having different mount options even in the same pid
namespace and this allows for the removal of proc_mnt which will
trivially the first mount of proc to honor it's mount options.
This flushing remains an optimization. The functions
pid_delete_dentry and pid_revalidate ensure that ordinary dcache
management will not attempt to use dentries past the point their
respective task has died. When unused the shrinker will
eventually be able to remove these dentries.
There is a case in de_thread where proc_flush_pid can be
called early for a given pid. Which winds up being
safe (if suboptimal) as this is just an optiimization.
Only pid directories are put on the list as the other
per pid files are children of those directories and
d_invalidate on the directory will get them as well.
So that the pid can be used during flushing it's reference count is
taken in release_task and dropped in proc_flush_pid. Further the call
of proc_flush_pid is moved after the tasklist_lock is released in
release_task so that it is certain that the pid has already been
unhashed when flushing it taking place. This removes a small race
where a dentry could recreated.
As struct pid is supposed to be small and I need a per pid lock
I reuse the only lock that currently exists in struct pid the
the wait_pidfd.lock.
The net result is that this adds all of this functionality
with just a little extra list management overhead and
a single extra pointer in struct pid.
v2: Initialize pid->inodes. I somehow failed to get that
initialization into the initial version of the patch. A boot
failure was reported by "kernel test robot <lkp@intel.com>", and
failure to initialize that pid->inodes matches all of the reported
symptoms.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Fixes: f333c700c610 ("pidns: Add a limit on the number of pid
namespaces")
Fixes: 60347f6716aa ("pid namespaces: prepare proc_flust_task() to flush
entries from multiple proc trees")
Cc: <stable@vger.kernel.org> # 4.9.x: b3e583825266: clone: add CLONE_PIDFD
Cc: <stable@vger.kernel.org> # 4.9.x: b53b0b9d9a61: pidfd: add polling
support
Cc: <stable@vger.kernel.org> # 4.9.x: db978da8fa1d: proc: Pass file mode to
proc_pid_make_inode
Cc: <stable@vger.kernel.org> # 4.9.x: 68eb94f16227: proc: Better ownership of
files for non-dumpable tasks in user namespaces
Cc: <stable@vger.kernel.org> # 4.9.x: e3912ac37e07: proc: use %u for pid
printing and slightly less stack
Cc: <stable@vger.kernel.org> # 4.9.x: 0afa5ca82212: proc: Rename in
proc_inode rename sysctl_inodes sibling_inodes
Cc: <stable@vger.kernel.org> # 4.9.x: 26dbc60f385f: proc: Generalize
proc_sys_prune_dcache into proc_prune_siblings_dcache
Cc: <stable@vger.kernel.org> # 4.9.x: 71448011ea2a: proc: Clear the pieces of
proc_inode that proc_evict_inode cares about
Cc: <stable@vger.kernel.org> # 4.9.x: f90f3cafe8d5: Use d_invalidate in
proc_prune_siblings_dcache
Cc: <stable@vger.kernel.org> # 4.9.x
(proc: fix up cherry-pick conflicts for 7bc3e6e55acf)
Signed-off-by: Wen Yang <wenyang@linux.alibaba.com>
---
fs/proc/base.c | 111 ++++++++++++++++--------------------------------
fs/proc/inode.c | 2 +-
fs/proc/internal.h | 1 +
include/linux/pid.h | 1 +
include/linux/proc_fs.h | 4 +-
kernel/exit.c | 5 ++-
kernel/pid.c | 1 +
7 files changed, 45 insertions(+), 80 deletions(-)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 3502a40..11caf35 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1728,11 +1728,25 @@ void task_dump_owner(struct task_struct *task, mode_t mode,
*rgid = gid;
}
+void proc_pid_evict_inode(struct proc_inode *ei)
+{
+ struct pid *pid = ei->pid;
+
+ if (S_ISDIR(ei->vfs_inode.i_mode)) {
+ spin_lock(&pid->wait_pidfd.lock);
+ hlist_del_init_rcu(&ei->sibling_inodes);
+ spin_unlock(&pid->wait_pidfd.lock);
+ }
+
+ put_pid(pid);
+}
+
struct inode *proc_pid_make_inode(struct super_block * sb,
struct task_struct *task, umode_t mode)
{
struct inode * inode;
struct proc_inode *ei;
+ struct pid *pid;
/* We need a new inode */
@@ -1750,10 +1764,18 @@ struct inode *proc_pid_make_inode(struct super_block * sb,
/*
* grab the reference to task.
*/
- ei->pid = get_task_pid(task, PIDTYPE_PID);
- if (!ei->pid)
+ pid = get_task_pid(task, PIDTYPE_PID);
+ if (!pid)
goto out_unlock;
+ /* Let the pid remember us for quick removal */
+ ei->pid = pid;
+ if (S_ISDIR(mode)) {
+ spin_lock(&pid->wait_pidfd.lock);
+ hlist_add_head_rcu(&ei->sibling_inodes, &pid->inodes);
+ spin_unlock(&pid->wait_pidfd.lock);
+ }
+
task_dump_owner(task, 0, &inode->i_uid, &inode->i_gid);
security_task_to_inode(task, inode);
@@ -3015,90 +3037,29 @@ static struct dentry *proc_tgid_base_lookup(struct inode *dir, struct dentry *de
.permission = proc_pid_permission,
};
-static void proc_flush_task_mnt(struct vfsmount *mnt, pid_t pid, pid_t tgid)
-{
- struct dentry *dentry, *leader, *dir;
- char buf[10 + 1];
- struct qstr name;
-
- name.name = buf;
- name.len = snprintf(buf, sizeof(buf), "%u", pid);
- /* no ->d_hash() rejects on procfs */
- dentry = d_hash_and_lookup(mnt->mnt_root, &name);
- if (dentry) {
- d_invalidate(dentry);
- dput(dentry);
- }
-
- if (pid == tgid)
- return;
-
- name.name = buf;
- name.len = snprintf(buf, sizeof(buf), "%u", tgid);
- leader = d_hash_and_lookup(mnt->mnt_root, &name);
- if (!leader)
- goto out;
-
- name.name = "task";
- name.len = strlen(name.name);
- dir = d_hash_and_lookup(leader, &name);
- if (!dir)
- goto out_put_leader;
-
- name.name = buf;
- name.len = snprintf(buf, sizeof(buf), "%u", pid);
- dentry = d_hash_and_lookup(dir, &name);
- if (dentry) {
- d_invalidate(dentry);
- dput(dentry);
- }
-
- dput(dir);
-out_put_leader:
- dput(leader);
-out:
- return;
-}
-
/**
- * proc_flush_task - Remove dcache entries for @task from the /proc dcache.
- * @task: task that should be flushed.
+ * proc_flush_pid - Remove dcache entries for @pid from the /proc dcache.
+ * @pid: pid that should be flushed.
*
- * When flushing dentries from proc, one needs to flush them from global
- * proc (proc_mnt) and from all the namespaces' procs this task was seen
- * in. This call is supposed to do all of this job.
- *
- * Looks in the dcache for
- * /proc/@pid
- * /proc/@tgid/task/@pid
- * if either directory is present flushes it and all of it'ts children
- * from the dcache.
+ * This function walks a list of inodes (that belong to any proc
+ * filesystem) that are attached to the pid and flushes them from
+ * the dentry cache.
*
* It is safe and reasonable to cache /proc entries for a task until
* that task exits. After that they just clog up the dcache with
* useless entries, possibly causing useful dcache entries to be
- * flushed instead. This routine is proved to flush those useless
- * dcache entries at process exit time.
+ * flushed instead. This routine is provided to flush those useless
+ * dcache entries when a process is reaped.
*
* NOTE: This routine is just an optimization so it does not guarantee
- * that no dcache entries will exist at process exit time it
- * just makes it very unlikely that any will persist.
+ * that no dcache entries will exist after a process is reaped
+ * it just makes it very unlikely that any will persist.
*/
-void proc_flush_task(struct task_struct *task)
+void proc_flush_pid(struct pid *pid)
{
- int i;
- struct pid *pid, *tgid;
- struct upid *upid;
-
- pid = task_pid(task);
- tgid = task_tgid(task);
-
- for (i = 0; i <= pid->level; i++) {
- upid = &pid->numbers[i];
- proc_flush_task_mnt(upid->ns->proc_mnt, upid->nr,
- tgid->numbers[i].nr);
- }
+ proc_invalidate_siblings_dcache(&pid->inodes, &pid->wait_pidfd.lock);
+ put_pid(pid);
}
static int proc_pid_instantiate(struct inode *dir,
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index 2af9f4f..8503444 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -39,7 +39,7 @@ static void proc_evict_inode(struct inode *inode)
/* Stop tracking associated processes */
if (ei->pid) {
- put_pid(ei->pid);
+ proc_pid_evict_inode(ei);
ei->pid = NULL;
}
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index 6a1d679..0c6ca639 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -151,6 +151,7 @@ extern int proc_pid_statm(struct seq_file *, struct pid_namespace *,
extern const struct dentry_operations pid_dentry_operations;
extern int pid_getattr(struct vfsmount *, struct dentry *, struct kstat *);
extern int proc_setattr(struct dentry *, struct iattr *);
+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 int pid_revalidate(struct dentry *, unsigned int);
extern int pid_delete_dentry(const struct dentry *);
diff --git a/include/linux/pid.h b/include/linux/pid.h
index f5552ba..04b4aaa 100644
--- a/include/linux/pid.h
+++ b/include/linux/pid.h
@@ -63,6 +63,7 @@ struct pid
unsigned int level;
/* lists of tasks that use this pid */
struct hlist_head tasks[PIDTYPE_MAX];
+ struct hlist_head inodes;
/* wait queue for pidfd notifications */
wait_queue_head_t wait_pidfd;
struct rcu_head rcu;
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index b97bf2e..d3580f5 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -12,7 +12,7 @@
#ifdef CONFIG_PROC_FS
extern void proc_root_init(void);
-extern void proc_flush_task(struct task_struct *);
+extern void proc_flush_pid(struct pid *);
extern struct proc_dir_entry *proc_symlink(const char *,
struct proc_dir_entry *, const char *);
@@ -48,7 +48,7 @@ static inline void proc_root_init(void)
{
}
-static inline void proc_flush_task(struct task_struct *task)
+static inline void proc_flush_pid(struct pid *pid)
{
}
diff --git a/kernel/exit.c b/kernel/exit.c
index f9943ef..5e66030 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -168,6 +168,7 @@ static void delayed_put_task_struct(struct rcu_head *rhp)
void release_task(struct task_struct *p)
{
struct task_struct *leader;
+ struct pid *thread_pid;
int zap_leader;
repeat:
/* don't need to get the RCU readlock here - the process is dead and
@@ -176,10 +177,9 @@ void release_task(struct task_struct *p)
atomic_dec(&__task_cred(p)->user->processes);
rcu_read_unlock();
- proc_flush_task(p);
-
write_lock_irq(&tasklist_lock);
ptrace_release_task(p);
+ thread_pid = get_pid(p->pids[PIDTYPE_PID].pid);
__exit_signal(p);
/*
@@ -202,6 +202,7 @@ void release_task(struct task_struct *p)
}
write_unlock_irq(&tasklist_lock);
+ proc_flush_pid(thread_pid);
release_thread(p);
call_rcu(&p->rcu, delayed_put_task_struct);
diff --git a/kernel/pid.c b/kernel/pid.c
index e605398..fb32a81 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -334,6 +334,7 @@ struct pid *alloc_pid(struct pid_namespace *ns)
INIT_HLIST_HEAD(&pid->tasks[type]);
init_waitqueue_head(&pid->wait_pidfd);
+ INIT_HLIST_HEAD(&pid->inodes);
upid = pid->numbers + ns->level;
spin_lock_irq(&pidmap_lock);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v2 4.9 00/10] fix a race in release_task when flushing the dentry
2021-01-07 7:52 [PATCH v2 4.9 00/10] fix a race in release_task when flushing the dentry Wen Yang
` (9 preceding siblings ...)
2021-01-07 7:52 ` [PATCH v2 4.9 10/10] proc: Use a list of inodes to flush from proc Wen Yang
@ 2021-01-07 12:17 ` Greg Kroah-Hartman
2021-01-07 16:21 ` Wen Yang
10 siblings, 1 reply; 17+ messages in thread
From: Greg Kroah-Hartman @ 2021-01-07 12:17 UTC (permalink / raw)
To: Wen Yang; +Cc: Sasha Levin, Xunlei Pang, linux-kernel
On Thu, Jan 07, 2021 at 03:52:12PM +0800, Wen Yang wrote:
> The dentries such as /proc/<pid>/ns/ have the DCACHE_OP_DELETE flag, they
> should be deleted when the process exits.
>
> Suppose the following race appears：
>
> release_task dput
> -> proc_flush_task
> -> dentry->d_op->d_delete(dentry)
> -> __exit_signal
> -> dentry->d_lockref.count-- and return.
>
> In the proc_flush_task(), if another process is using this dentry, it will
> not be deleted. At the same time, in dput(), d_op->d_delete() can be executed
> before __exit_signal(pid has not been hashed), d_delete returns false, so
> this dentry still cannot be deleted.
>
> This dentry will always be cached (although its count is 0 and the
> DCACHE_OP_DELETE flag is set), its parent denry will also be cached too, and
> these dentries can only be deleted when drop_caches is manually triggered.
>
> This will result in wasted memory. What's more troublesome is that these
> dentries reference pid, according to the commit f333c700c610 ("pidns: Add a
> limit on the number of pid namespaces"), if the pid cannot be released, it
> may result in the inability to create a new pid_ns.
>
> This issue was introduced by 60347f6716aa ("pid namespaces: prepare
> proc_flust_task() to flush entries from multiple proc trees"), exposed by
> f333c700c610 ("pidns: Add a limit on the number of pid namespaces"), and then
> fixed by 7bc3e6e55acf ("proc: Use a list of inodes to flush from proc").
Why are you just submitting a series for 4.9 and 4.19, what about 4.14?
We can't have users move to a newer kernel and then experience old bugs,
right?
But the larger question is why are you backporting a whole new feature
here? Why is CLONE_PIDFD needed? That feels really wrong...
thanks,
greg k-h
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2 4.9 00/10] fix a race in release_task when flushing the dentry
2021-01-07 12:17 ` [PATCH v2 4.9 00/10] fix a race in release_task when flushing the dentry Greg Kroah-Hartman
@ 2021-01-07 16:21 ` Wen Yang
2021-01-07 18:28 ` Greg Kroah-Hartman
0 siblings, 1 reply; 17+ messages in thread
From: Wen Yang @ 2021-01-07 16:21 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Sasha Levin, Xunlei Pang, linux-kernel
在 2021/1/7 下午8:17, Greg Kroah-Hartman 写道:
> On Thu, Jan 07, 2021 at 03:52:12PM +0800, Wen Yang wrote:
>> The dentries such as /proc/<pid>/ns/ have the DCACHE_OP_DELETE flag, they
>> should be deleted when the process exits.
>>
>> Suppose the following race appears：
>>
>> release_task dput
>> -> proc_flush_task
>> -> dentry->d_op->d_delete(dentry)
>> -> __exit_signal
>> -> dentry->d_lockref.count-- and return.
>>
>> In the proc_flush_task(), if another process is using this dentry, it will
>> not be deleted. At the same time, in dput(), d_op->d_delete() can be executed
>> before __exit_signal(pid has not been hashed), d_delete returns false, so
>> this dentry still cannot be deleted.
>>
>> This dentry will always be cached (although its count is 0 and the
>> DCACHE_OP_DELETE flag is set), its parent denry will also be cached too, and
>> these dentries can only be deleted when drop_caches is manually triggered.
>>
>> This will result in wasted memory. What's more troublesome is that these
>> dentries reference pid, according to the commit f333c700c610 ("pidns: Add a
>> limit on the number of pid namespaces"), if the pid cannot be released, it
>> may result in the inability to create a new pid_ns.
>>
>> This issue was introduced by 60347f6716aa ("pid namespaces: prepare
>> proc_flust_task() to flush entries from multiple proc trees"), exposed by
>> f333c700c610 ("pidns: Add a limit on the number of pid namespaces"), and then
>> fixed by 7bc3e6e55acf ("proc: Use a list of inodes to flush from proc").
>
> Why are you just submitting a series for 4.9 and 4.19, what about 4.14?
> We can't have users move to a newer kernel and then experience old bugs,
> right?
>
Okay, the patches corresponding to 4.14 will be ready later.
> But the larger question is why are you backporting a whole new feature
> here? Why is CLONE_PIDFD needed? That feels really wrong...
>
The reason for backporting CLONE_PIDFD is because 7bc3e6e55acf ("proc:
Use a list of inodes to flush from proc") relies on wait_pidfd.lock.
There are indeed many associated modifications here. We are also testing
it. Please check the code more.
Thanks.
--
Best wishes,
Wen
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2 4.9 00/10] fix a race in release_task when flushing the dentry
2021-01-07 16:21 ` Wen Yang
@ 2021-01-07 18:28 ` Greg Kroah-Hartman
2021-01-08 2:42 ` Wen Yang
0 siblings, 1 reply; 17+ messages in thread
From: Greg Kroah-Hartman @ 2021-01-07 18:28 UTC (permalink / raw)
To: Wen Yang, Christian Brauner; +Cc: Sasha Levin, Xunlei Pang, linux-kernel
On Fri, Jan 08, 2021 at 12:21:38AM +0800, Wen Yang wrote:
>
>
> 在 2021/1/7 下午8:17, Greg Kroah-Hartman 写道:
> > On Thu, Jan 07, 2021 at 03:52:12PM +0800, Wen Yang wrote:
> > > The dentries such as /proc/<pid>/ns/ have the DCACHE_OP_DELETE flag, they
> > > should be deleted when the process exits.
> > >
> > > Suppose the following race appears：
> > >
> > > release_task dput
> > > -> proc_flush_task
> > > -> dentry->d_op->d_delete(dentry)
> > > -> __exit_signal
> > > -> dentry->d_lockref.count-- and return.
> > >
> > > In the proc_flush_task(), if another process is using this dentry, it will
> > > not be deleted. At the same time, in dput(), d_op->d_delete() can be executed
> > > before __exit_signal(pid has not been hashed), d_delete returns false, so
> > > this dentry still cannot be deleted.
> > >
> > > This dentry will always be cached (although its count is 0 and the
> > > DCACHE_OP_DELETE flag is set), its parent denry will also be cached too, and
> > > these dentries can only be deleted when drop_caches is manually triggered.
> > >
> > > This will result in wasted memory. What's more troublesome is that these
> > > dentries reference pid, according to the commit f333c700c610 ("pidns: Add a
> > > limit on the number of pid namespaces"), if the pid cannot be released, it
> > > may result in the inability to create a new pid_ns.
> > >
> > > This issue was introduced by 60347f6716aa ("pid namespaces: prepare
> > > proc_flust_task() to flush entries from multiple proc trees"), exposed by
> > > f333c700c610 ("pidns: Add a limit on the number of pid namespaces"), and then
> > > fixed by 7bc3e6e55acf ("proc: Use a list of inodes to flush from proc").
> >
> > Why are you just submitting a series for 4.9 and 4.19, what about 4.14?
> > We can't have users move to a newer kernel and then experience old bugs,
> > right?
> >
> Okay, the patches corresponding to 4.14 will be ready later.
Note for some reason you didn't cc: the stable list for these patches :(
> > But the larger question is why are you backporting a whole new feature
> > here? Why is CLONE_PIDFD needed? That feels really wrong...
> >
>
> The reason for backporting CLONE_PIDFD is because 7bc3e6e55acf ("proc: Use a
> list of inodes to flush from proc") relies on wait_pidfd.lock. There are
> indeed many associated modifications here. We are also testing it. Please
> check the code more.
Is the only "issue" here wasted memory? Will it eventually be freed
anyway even if you do not echo to the proc file to flush caches?
You mention the inability to create a new pid for a specific namespace,
is that really a problem? Shouldn't the code handle such issues
normally? What breaks without these changes?
I think at this point, it might just time for you to move to a newer
kernel release, as adding a whole new userspace feature for this feels
really really odd.
What is preventing you from doing that today? What holds you to older
kernels that will not allow you to move forward?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2 4.9 00/10] fix a race in release_task when flushing the dentry
2021-01-07 18:28 ` Greg Kroah-Hartman
@ 2021-01-08 2:42 ` Wen Yang
2021-01-09 12:36 ` Greg Kroah-Hartman
0 siblings, 1 reply; 17+ messages in thread
From: Wen Yang @ 2021-01-08 2:42 UTC (permalink / raw)
To: Greg Kroah-Hartman, Christian Brauner
Cc: Sasha Levin, Xunlei Pang, linux-kernel
在 2021/1/8 上午2:28, Greg Kroah-Hartman 写道:
> On Fri, Jan 08, 2021 at 12:21:38AM +0800, Wen Yang wrote:
>>
>>
>> 在 2021/1/7 下午8:17, Greg Kroah-Hartman 写道:
>>> On Thu, Jan 07, 2021 at 03:52:12PM +0800, Wen Yang wrote:
>>>> The dentries such as /proc/<pid>/ns/ have the DCACHE_OP_DELETE flag, they
>>>> should be deleted when the process exits.
>>>>
>>>> Suppose the following race appears：
>>>>
>>>> release_task dput
>>>> -> proc_flush_task
>>>> -> dentry->d_op->d_delete(dentry)
>>>> -> __exit_signal
>>>> -> dentry->d_lockref.count-- and return.
>>>>
>>>> In the proc_flush_task(), if another process is using this dentry, it will
>>>> not be deleted. At the same time, in dput(), d_op->d_delete() can be executed
>>>> before __exit_signal(pid has not been hashed), d_delete returns false, so
>>>> this dentry still cannot be deleted.
>>>>
>>>> This dentry will always be cached (although its count is 0 and the
>>>> DCACHE_OP_DELETE flag is set), its parent denry will also be cached too, and
>>>> these dentries can only be deleted when drop_caches is manually triggered.
>>>>
>>>> This will result in wasted memory. What's more troublesome is that these
>>>> dentries reference pid, according to the commit f333c700c610 ("pidns: Add a
>>>> limit on the number of pid namespaces"), if the pid cannot be released, it
>>>> may result in the inability to create a new pid_ns.
>>>>
>>>> This issue was introduced by 60347f6716aa ("pid namespaces: prepare
>>>> proc_flust_task() to flush entries from multiple proc trees"), exposed by
>>>> f333c700c610 ("pidns: Add a limit on the number of pid namespaces"), and then
>>>> fixed by 7bc3e6e55acf ("proc: Use a list of inodes to flush from proc").
>>>
>>> Why are you just submitting a series for 4.9 and 4.19, what about 4.14?
>>> We can't have users move to a newer kernel and then experience old bugs,
>>> right?
>>>
>> Okay, the patches corresponding to 4.14 will be ready later.
>
> Note for some reason you didn't cc: the stable list for these patches :(
>
>>> But the larger question is why are you backporting a whole new feature
>>> here? Why is CLONE_PIDFD needed? That feels really wrong...
>>>
>>
>> The reason for backporting CLONE_PIDFD is because 7bc3e6e55acf ("proc: Use a
>> list of inodes to flush from proc") relies on wait_pidfd.lock. There are
>> indeed many associated modifications here. We are also testing it. Please
>> check the code more.
>
> Is the only "issue" here wasted memory? Will it eventually be freed
> anyway even if you do not echo to the proc file to flush caches?
>
> You mention the inability to create a new pid for a specific namespace,
> is that really a problem? Shouldn't the code handle such issues
> normally? What breaks without these changes?
>
> I think at this point, it might just time for you to move to a newer
> kernel release, as adding a whole new userspace feature for this feels
> really really odd.
>
> What is preventing you from doing that today? What holds you to older
> kernels that will not allow you to move forward?
>
We have encountered this problem in the cloud server environment. Users
will frequently create and delete containers, and the corresponding
pid_ns will accumulate, eventually making it impossible to create a new
container.
https://bugzilla.kernel.org/show_bug.cgi?id=208613
The kernels (4.9/4.19) used on a large scale in our current production
environment (almost tens of thousands of machines) may need to be fixed.
Thanks.
--
Best wishes,
Wen
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2 4.9 00/10] fix a race in release_task when flushing the dentry
2021-01-08 2:42 ` Wen Yang
@ 2021-01-09 12:36 ` Greg Kroah-Hartman
0 siblings, 0 replies; 17+ messages in thread
From: Greg Kroah-Hartman @ 2021-01-09 12:36 UTC (permalink / raw)
To: Wen Yang; +Cc: Christian Brauner, Sasha Levin, Xunlei Pang, linux-kernel
On Fri, Jan 08, 2021 at 10:42:47AM +0800, Wen Yang wrote:
>
>
> 在 2021/1/8 上午2:28, Greg Kroah-Hartman 写道:
> > On Fri, Jan 08, 2021 at 12:21:38AM +0800, Wen Yang wrote:
> > >
> > >
> > > 在 2021/1/7 下午8:17, Greg Kroah-Hartman 写道:
> > > > On Thu, Jan 07, 2021 at 03:52:12PM +0800, Wen Yang wrote:
> > > > > The dentries such as /proc/<pid>/ns/ have the DCACHE_OP_DELETE flag, they
> > > > > should be deleted when the process exits.
> > > > >
> > > > > Suppose the following race appears：
> > > > >
> > > > > release_task dput
> > > > > -> proc_flush_task
> > > > > -> dentry->d_op->d_delete(dentry)
> > > > > -> __exit_signal
> > > > > -> dentry->d_lockref.count-- and return.
> > > > >
> > > > > In the proc_flush_task(), if another process is using this dentry, it will
> > > > > not be deleted. At the same time, in dput(), d_op->d_delete() can be executed
> > > > > before __exit_signal(pid has not been hashed), d_delete returns false, so
> > > > > this dentry still cannot be deleted.
> > > > >
> > > > > This dentry will always be cached (although its count is 0 and the
> > > > > DCACHE_OP_DELETE flag is set), its parent denry will also be cached too, and
> > > > > these dentries can only be deleted when drop_caches is manually triggered.
> > > > >
> > > > > This will result in wasted memory. What's more troublesome is that these
> > > > > dentries reference pid, according to the commit f333c700c610 ("pidns: Add a
> > > > > limit on the number of pid namespaces"), if the pid cannot be released, it
> > > > > may result in the inability to create a new pid_ns.
> > > > >
> > > > > This issue was introduced by 60347f6716aa ("pid namespaces: prepare
> > > > > proc_flust_task() to flush entries from multiple proc trees"), exposed by
> > > > > f333c700c610 ("pidns: Add a limit on the number of pid namespaces"), and then
> > > > > fixed by 7bc3e6e55acf ("proc: Use a list of inodes to flush from proc").
> > > >
> > > > Why are you just submitting a series for 4.9 and 4.19, what about 4.14?
> > > > We can't have users move to a newer kernel and then experience old bugs,
> > > > right?
> > > >
> > > Okay, the patches corresponding to 4.14 will be ready later.
> >
> > Note for some reason you didn't cc: the stable list for these patches :(
> >
> > > > But the larger question is why are you backporting a whole new feature
> > > > here? Why is CLONE_PIDFD needed? That feels really wrong...
> > > >
> > >
> > > The reason for backporting CLONE_PIDFD is because 7bc3e6e55acf ("proc: Use a
> > > list of inodes to flush from proc") relies on wait_pidfd.lock. There are
> > > indeed many associated modifications here. We are also testing it. Please
> > > check the code more.
> >
> > Is the only "issue" here wasted memory? Will it eventually be freed
> > anyway even if you do not echo to the proc file to flush caches?
> >
> > You mention the inability to create a new pid for a specific namespace,
> > is that really a problem? Shouldn't the code handle such issues
> > normally? What breaks without these changes?
> >
> > I think at this point, it might just time for you to move to a newer
> > kernel release, as adding a whole new userspace feature for this feels
> > really really odd.
> >
> > What is preventing you from doing that today? What holds you to older
> > kernels that will not allow you to move forward?
> >
>
> We have encountered this problem in the cloud server environment. Users will
> frequently create and delete containers, and the corresponding pid_ns will
> accumulate, eventually making it impossible to create a new container.
>
> https://bugzilla.kernel.org/show_bug.cgi?id=208613
>
> The kernels (4.9/4.19) used on a large scale in our current production
> environment (almost tens of thousands of machines) may need to be fixed.
What prevents you from moving them to 5.4 or better yet, 5.10? You will
have to do it soon anyway, I'm sure you have been testing those kernels
to validate that all works well with them on a subset of your
environment, so for those systems that have this problem, why can't you
update the base kernel?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 17+ messages in thread