From f869e8a7f753e3fd43d6483e796774776f645edb Mon Sep 17 00:00:00 2001 From: Al Viro Date: Wed, 15 Aug 2012 21:06:33 -0400 Subject: [PATCH] expose a low-level variant of fd_install() for binder Similar situation to that of __alloc_fd(); do not use unless you really have to. You should not touch any descriptor table other than your own; it's a sure sign of a really bad API design. As with __alloc_fd(), you *must* use a first-class reference to struct files_struct; something obtained by get_files_struct(some task) (let alone direct task->files) will not do. It must be either current->files, or obtained by get_files_struct(current) by the owner of that sucker and given to you. Signed-off-by: Al Viro --- drivers/staging/android/binder.c | 13 ++----------- fs/file.c | 16 ++++++++++++++-- include/linux/fdtable.h | 2 ++ 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c index 4946d282a35..9e1a98a360d 100644 --- a/drivers/staging/android/binder.c +++ b/drivers/staging/android/binder.c @@ -386,17 +386,8 @@ int task_get_unused_fd_flags(struct binder_proc *proc, int flags) static void task_fd_install( struct binder_proc *proc, unsigned int fd, struct file *file) { - struct files_struct *files = proc->files; - struct fdtable *fdt; - - if (files == NULL) - return; - - spin_lock(&files->file_lock); - fdt = files_fdtable(files); - BUG_ON(fdt->fd[fd] != NULL); - rcu_assign_pointer(fdt->fd[fd], file); - spin_unlock(&files->file_lock); + if (proc->files) + __fd_install(proc->files, fd, file); } /* diff --git a/fs/file.c b/fs/file.c index 78cf88f2a0e..0d1bf051511 100644 --- a/fs/file.c +++ b/fs/file.c @@ -599,11 +599,18 @@ EXPORT_SYMBOL(put_unused_fd); * * It should never happen - if we allow dup2() do it, _really_ bad things * will follow. + * + * NOTE: __fd_install() variant is really, really low-level; don't + * use it unless you are forced to by truly lousy API shoved down + * your throat. 'files' *MUST* be either current->files or obtained + * by get_files_struct(current) done by whoever had given it to you, + * or really bad things will happen. Normally you want to use + * fd_install() instead. */ -void fd_install(unsigned int fd, struct file *file) +void __fd_install(struct files_struct *files, unsigned int fd, + struct file *file) { - struct files_struct *files = current->files; struct fdtable *fdt; spin_lock(&files->file_lock); fdt = files_fdtable(files); @@ -612,4 +619,9 @@ void fd_install(unsigned int fd, struct file *file) spin_unlock(&files->file_lock); } +void fd_install(unsigned int fd, struct file *file) +{ + __fd_install(current->files, fd, file); +} + EXPORT_SYMBOL(fd_install); diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h index 3855f4febe7..59d4fc7f10c 100644 --- a/include/linux/fdtable.h +++ b/include/linux/fdtable.h @@ -121,6 +121,8 @@ struct files_struct *dup_fd(struct files_struct *, int *); extern int __alloc_fd(struct files_struct *files, unsigned start, unsigned end, unsigned flags); +extern void __fd_install(struct files_struct *files, + unsigned int fd, struct file *file); extern struct kmem_cache *files_cachep; -- 2.46.0