| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20220531153708.3449446-1-chengzhihao1@huawei.com>
Date: Tue, 31 May 2022 23:37:08 +0800
From: Zhihao Cheng <chengzhihao1@...wei.com>
To: <ebiederm@...ssion.com>
CC: <linux-kernel@...r.kernel.org>, <linux-fsdevel@...r.kernel.org>,
<chengzhihao1@...wei.com>, <yukuai3@...wei.com>,
<yi.zhang@...wei.com>
Subject: [PATCH RFC] proc: Fix a dentry lock race between release_task and lookup
Commit 7bc3e6e55acf06 ("proc: Use a list of inodes to flush from proc")
moved proc_flush_task() behind __exit_signal(). Then, process systemd
can take long period high cpu usage during releasing task in following
concurrent processes:
systemd ps
kernel_waitid stat(/proc/pid)
do_wait filename_lookup
wait_consider_task lookup_fast
release_task
__exit_signal
__unhash_process
detach_pid
__change_pid // remove task->pid_links
d_revalidate -> pid_revalidate // 0
d_invalidate(/proc/pid)
shrink_dcache_parent(/proc/pid)
d_walk(/proc/pid)
spin_lock_nested(/proc/pid/fd)
// iterating opened fd
proc_flush_pid |
d_invalidate (/proc/pid/fd) |
shrink_dcache_parent(/proc/pid/fd) |
shrink_dentry_list(subdirs) ↓
shrink_lock_dentry(/proc/pid/fd) ---> race on dentry lock
Function d_invalidate() will remove dentry from hash firstly, but why does
proc_flush_pid() process dentry '/proc/pid/fd' before dentry '/proc/pid'?
That's because proc_pid_make_inode() adds proc inode in reverse order by
invoking hlist_add_head_rcu(). But proc should not add any inodes under
'/proc/pid' except '/proc/pid/task/pid', fix it by adding inode into
'pid->inodes' only if the inode is /proc/pid or /proc/pid/task/pid.
Fixes: 7bc3e6e55acf06 ("proc: Use a list of inodes to flush from proc")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=216054
Signed-off-by: Zhihao Cheng <chengzhihao1@...wei.com>
Signed-off-by: Zhang Yi <yi.zhang@...wei.com>
---
fs/proc/base.c | 15 +++++++++------
fs/proc/fd.c | 4 ++--
fs/proc/internal.h | 3 ++-
fs/proc/namespaces.c | 3 ++-
4 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index c1031843cc6a..29c0f1175766 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1886,7 +1886,8 @@ void proc_pid_evict_inode(struct proc_inode *ei)
}
struct inode *proc_pid_make_inode(struct super_block * sb,
- struct task_struct *task, umode_t mode)
+ struct task_struct *task,
+ umode_t mode, bool add_inode)
{
struct inode * inode;
struct proc_inode *ei;
@@ -1914,7 +1915,7 @@ struct inode *proc_pid_make_inode(struct super_block * sb,
/* Let the pid remember us for quick removal */
ei->pid = pid;
- if (S_ISDIR(mode)) {
+ if (add_inode) {
spin_lock(&pid->lock);
hlist_add_head_rcu(&ei->sibling_inodes, &pid->inodes);
spin_unlock(&pid->lock);
@@ -2243,7 +2244,7 @@ proc_map_files_instantiate(struct dentry *dentry,
inode = proc_pid_make_inode(dentry->d_sb, task, S_IFLNK |
((mode & FMODE_READ ) ? S_IRUSR : 0) |
- ((mode & FMODE_WRITE) ? S_IWUSR : 0));
+ ((mode & FMODE_WRITE) ? S_IWUSR : 0), false);
if (!inode)
return ERR_PTR(-ENOENT);
@@ -2609,7 +2610,7 @@ static struct dentry *proc_pident_instantiate(struct dentry *dentry,
struct inode *inode;
struct proc_inode *ei;
- inode = proc_pid_make_inode(dentry->d_sb, task, p->mode);
+ inode = proc_pid_make_inode(dentry->d_sb, task, p->mode, false);
if (!inode)
return ERR_PTR(-ENOENT);
@@ -3350,7 +3351,8 @@ static struct dentry *proc_pid_instantiate(struct dentry * dentry,
{
struct inode *inode;
- inode = proc_pid_make_inode(dentry->d_sb, task, S_IFDIR | S_IRUGO | S_IXUGO);
+ inode = proc_pid_make_inode(dentry->d_sb, task,
+ S_IFDIR | S_IRUGO | S_IXUGO, true);
if (!inode)
return ERR_PTR(-ENOENT);
@@ -3649,7 +3651,8 @@ static struct dentry *proc_task_instantiate(struct dentry *dentry,
struct task_struct *task, const void *ptr)
{
struct inode *inode;
- inode = proc_pid_make_inode(dentry->d_sb, task, S_IFDIR | S_IRUGO | S_IXUGO);
+ inode = proc_pid_make_inode(dentry->d_sb, task,
+ S_IFDIR | S_IRUGO | S_IXUGO, true);
if (!inode)
return ERR_PTR(-ENOENT);
diff --git a/fs/proc/fd.c b/fs/proc/fd.c
index 913bef0d2a36..53365ebb4567 100644
--- a/fs/proc/fd.c
+++ b/fs/proc/fd.c
@@ -199,7 +199,7 @@ static struct dentry *proc_fd_instantiate(struct dentry *dentry,
struct proc_inode *ei;
struct inode *inode;
- inode = proc_pid_make_inode(dentry->d_sb, task, S_IFLNK);
+ inode = proc_pid_make_inode(dentry->d_sb, task, S_IFLNK, false);
if (!inode)
return ERR_PTR(-ENOENT);
@@ -332,7 +332,7 @@ static struct dentry *proc_fdinfo_instantiate(struct dentry *dentry,
struct proc_inode *ei;
struct inode *inode;
- inode = proc_pid_make_inode(dentry->d_sb, task, S_IFREG | S_IRUSR);
+ inode = proc_pid_make_inode(dentry->d_sb, task, S_IFREG | S_IRUSR, false);
if (!inode)
return ERR_PTR(-ENOENT);
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index 06a80f78433d..9894a591a38c 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -162,7 +162,8 @@ extern int pid_getattr(struct user_namespace *, const struct path *,
extern int pid_getattr(const struct path *, struct kstat *, u32, unsigned int);
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 struct inode *proc_pid_make_inode(struct super_block *,
+ struct task_struct *, umode_t, bool);
extern void pid_update_inode(struct task_struct *, struct inode *);
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 8e159fc78c0a..c9006e3b36fc 100644
--- a/fs/proc/namespaces.c
+++ b/fs/proc/namespaces.c
@@ -102,7 +102,8 @@ static struct dentry *proc_ns_instantiate(struct dentry *dentry,
struct inode *inode;
struct proc_inode *ei;
- inode = proc_pid_make_inode(dentry->d_sb, task, S_IFLNK | S_IRWXUGO);
+ inode = proc_pid_make_inode(dentry->d_sb, task, S_IFLNK | S_IRWXUGO,
+ false);
if (!inode)
return ERR_PTR(-ENOENT);
--
2.31.1
Powered by blists - more mailing lists