diff options
author | Oleg Nesterov <oleg@redhat.com> | 2014-01-11 19:19:32 +0100 |
---|---|---|
committer | Al Viro <viro@zeniv.linux.org.uk> | 2014-01-25 03:14:36 -0500 |
commit | a8d4b8345e0ee48b732126d980efaf0dc373e2b0 (patch) | |
tree | 4bdfc52ef4ad176f54e4ce7b81091c0fbef0fd04 /include/linux/fdtable.h | |
parent | 2ccdc413196b43a02bb68b46be5b68850904e9ea (diff) | |
download | linux-stable-a8d4b8345e0ee48b732126d980efaf0dc373e2b0.tar.gz linux-stable-a8d4b8345e0ee48b732126d980efaf0dc373e2b0.tar.bz2 linux-stable-a8d4b8345e0ee48b732126d980efaf0dc373e2b0.zip |
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 turns fcheck_files() into
__fcheck_files() which uses rcu_dereference_raw(), the "unshared"
callers, fget_light() and fget_raw_light(), can use it to avoid
the warning from RCU-lockdep.
fcheck_files() is trivially reimplemented as rcu_lockdep_assert()
plus __fcheck_files().
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Diffstat (limited to 'include/linux/fdtable.h')
-rw-r--r-- | include/linux/fdtable.h | 35 |
1 files changed, 21 insertions, 14 deletions
diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h index 085197bd8812..70e8e21c0a30 100644 --- a/include/linux/fdtable.h +++ b/include/linux/fdtable.h @@ -59,29 +59,36 @@ struct files_struct { struct file __rcu * fd_array[NR_OPEN_DEFAULT]; }; -#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())) - -#define files_fdtable(files) \ - (rcu_dereference_check_fdtable((files), (files)->fdt)) - struct file_operations; struct vfsmount; struct dentry; extern void __init files_defer_init(void); -static inline struct file * fcheck_files(struct files_struct *files, unsigned int fd) +#define rcu_dereference_check_fdtable(files, fdtfd) \ + rcu_dereference_check((fdtfd), lockdep_is_held(&(files)->file_lock)) + +#define files_fdtable(files) \ + rcu_dereference_check_fdtable((files), (files)->fdt) + +/* + * The caller must ensure that fd table isn't shared or hold rcu or file lock + */ +static inline struct file *__fcheck_files(struct files_struct *files, unsigned int fd) { - struct file * file = NULL; - struct fdtable *fdt = files_fdtable(files); + struct fdtable *fdt = rcu_dereference_raw(files->fdt); if (fd < fdt->max_fds) - file = rcu_dereference_check_fdtable(files, fdt->fd[fd]); - return file; + return rcu_dereference_raw(fdt->fd[fd]); + return NULL; +} + +static inline struct file *fcheck_files(struct files_struct *files, unsigned int fd) +{ + rcu_lockdep_assert(rcu_read_lock_held() || + lockdep_is_held(&files->file_lock), + "suspicious rcu_dereference_check() usage"); + return __fcheck_files(files, fd); } /* |