| 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: <20140107181317.GA29306@redhat.com>
Date: Tue, 7 Jan 2014 19:13:17 +0100
From: Oleg Nesterov <oleg@...hat.com>
To: Andrew Morton <akpm@...ux-foundation.org>,
"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Cc: Al Viro <viro@...IV.linux.org.uk>,
Dipankar Sarma <dipankar@...ibm.com>,
Eric Dumazet <edumazet@...gle.com>,
linux-kernel@...r.kernel.org
Subject: [PATCH 1/2] introduce __fcheck_files() to fix
rcu_dereference_check_fdtable(), kill rcu_my_thread_group_empty()
rcu_dereference_check_fdtable() looks very wrong,
1. rcu_my_thread_group_empty() was added by 844b9a8707f1 "vfs: fix
RCU-lockdep false positive due to /proc" but it doesn't really
fix the problem. A CLONE_THREAD (without CLONE_FILES) task can
hit the same race with get_files_struct().
And otoh rcu_my_thread_group_empty() can suppress the correct
warning if the caller is the CLONE_FILES (without CLONE_THREAD)
task.
2. files->count == 1 check is not really right too. Even if this
files_struct is not shared it is not safe to access it lockless
unless the caller is the owner.
Otoh, this check is sub-optimal. files->count == 0 always means
it is safe to use it lockless even if files != current->files,
but put_files_struct() has to take rcu_read_lock(). See the next
patch.
This patch removes the buggy checks and adds the new helper which
simply does rcu_lock_acquire/release around fcheck_files(). The
files->count == 1 callers, fget_light() and fget_raw_light(), can
use this helper to avoid the warning from RCU-lockdep.
Signed-off-by: Oleg Nesterov <oleg@...hat.com>
---
fs/file.c | 4 ++--
include/linux/fdtable.h | 19 +++++++++++++------
include/linux/rcupdate.h | 2 --
kernel/rcu/update.c | 11 -----------
4 files changed, 15 insertions(+), 21 deletions(-)
diff --git a/fs/file.c b/fs/file.c
index 4a78f98..957cbc0 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -707,7 +707,7 @@ struct file *fget_light(unsigned int fd, int *fput_needed)
*fput_needed = 0;
if (atomic_read(&files->count) == 1) {
- file = fcheck_files(files, fd);
+ file = __fcheck_files(files, fd);
if (file && (file->f_mode & FMODE_PATH))
file = NULL;
} else {
@@ -735,7 +735,7 @@ struct file *fget_raw_light(unsigned int fd, int *fput_needed)
*fput_needed = 0;
if (atomic_read(&files->count) == 1) {
- file = fcheck_files(files, fd);
+ file = __fcheck_files(files, fd);
} else {
rcu_read_lock();
file = fcheck_files(files, fd);
diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h
index 085197b..f39d50a 100644
--- a/include/linux/fdtable.h
+++ b/include/linux/fdtable.h
@@ -60,10 +60,7 @@ struct files_struct {
};
#define rcu_dereference_check_fdtable(files, fdtfd) \
- (rcu_dereference_check((fdtfd), \
- lockdep_is_held(&(files)->file_lock) || \
- atomic_read(&(files)->count) == 1 || \
- rcu_my_thread_group_empty()))
+ rcu_dereference_check((fdtfd), lockdep_is_held(&(files)->file_lock))
#define files_fdtable(files) \
(rcu_dereference_check_fdtable((files), (files)->fdt))
@@ -74,9 +71,9 @@ struct dentry;
extern void __init files_defer_init(void);
-static inline struct file * fcheck_files(struct files_struct *files, unsigned int fd)
+static inline struct file *fcheck_files(struct files_struct *files, unsigned int fd)
{
- struct file * file = NULL;
+ struct file *file = NULL;
struct fdtable *fdt = files_fdtable(files);
if (fd < fdt->max_fds)
@@ -84,6 +81,16 @@ static inline struct file * fcheck_files(struct files_struct *files, unsigned in
return file;
}
+/* The caller must ensure that fd table isn't shared */
+static inline struct file *__fcheck_files(struct files_struct *files, unsigned int fd)
+{
+ struct file *file;
+ /* make rcu_dereference_check_fdtable() happy */
+ rcu_lock_acquire(&rcu_lock_map);
+ file = fcheck_files(files, fd);
+ rcu_lock_release(&rcu_lock_map);
+ return file;
+}
/*
* Check whether the specified fd has an open file.
*/
diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 39cbb88..a2482cf 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -448,8 +448,6 @@ static inline int rcu_read_lock_sched_held(void)
#ifdef CONFIG_PROVE_RCU
-extern int rcu_my_thread_group_empty(void);
-
/**
* rcu_lockdep_assert - emit lockdep splat if specified condition not met
* @c: condition to check
diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c
index 6cb3dff..a3596c8 100644
--- a/kernel/rcu/update.c
+++ b/kernel/rcu/update.c
@@ -195,17 +195,6 @@ void wait_rcu_gp(call_rcu_func_t crf)
}
EXPORT_SYMBOL_GPL(wait_rcu_gp);
-#ifdef CONFIG_PROVE_RCU
-/*
- * wrapper function to avoid #include problems.
- */
-int rcu_my_thread_group_empty(void)
-{
- return thread_group_empty(current);
-}
-EXPORT_SYMBOL_GPL(rcu_my_thread_group_empty);
-#endif /* #ifdef CONFIG_PROVE_RCU */
-
#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
static inline void debug_init_rcu_head(struct rcu_head *head)
{
--
1.5.5.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists