Skip to content

Commit

Permalink
Staging: binder: Keep a reference to the files_struct while the drive…
Browse files Browse the repository at this point in the history 
…r is mmapped

This prevents breaking fget_light if a single threaded application
allows incoming file descriptors (in replies or on nodes).
Should also prevent inserting a file in the wrong files_struct if the
receving process execs in the middle of a transaction (between
task_get_unused_fd_flags and task_fd_install).

Signed-off-by: Arve Hjønnevåg <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
  • Loading branch information
@arve-android @gregkh
arve-android authored and gregkh committed Apr 17, 2009
1 parent ea5c4cc commit 282ca17
Showing 1 changed file with 63 additions and 16 deletions.
79 changes: 63 additions & 16 deletions drivers/staging/android/binder.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ static int binder_last_id;
static struct proc_dir_entry *binder_proc_dir_entry_root;
static struct proc_dir_entry *binder_proc_dir_entry_proc;
static struct hlist_head binder_dead_nodes;
static HLIST_HEAD(binder_release_files_list);
static DEFINE_MUTEX(binder_release_files_lock);

static int binder_read_proc_proc(
char *page, char **start, off_t off, int count, int *eof, void *data);
Expand Down Expand Up @@ -241,6 +243,8 @@ struct binder_proc {
int pid;
struct vm_area_struct *vma;
struct task_struct *tsk;
struct files_struct *files;
struct hlist_node release_files_node;
void *buffer;
size_t user_buffer_offset;

Expand Down Expand Up @@ -309,9 +313,9 @@ struct binder_transaction {
/*
* copied from get_unused_fd_flags
*/
int task_get_unused_fd_flags(struct task_struct *tsk, int flags)
int task_get_unused_fd_flags(struct binder_proc *proc, int flags)
{
struct files_struct *files = get_files_struct(tsk);
struct files_struct *files = proc->files;
int fd, error;
struct fdtable *fdt;
unsigned long rlim_cur;
Expand All @@ -333,9 +337,9 @@ int task_get_unused_fd_flags(struct task_struct *tsk, int flags)
* will limit the total number of files that can be opened.
*/
rlim_cur = 0;
if (lock_task_sighand(tsk, &irqs)) {
rlim_cur = tsk->signal->rlim[RLIMIT_NOFILE].rlim_cur;
unlock_task_sighand(tsk, &irqs);
if (lock_task_sighand(proc->tsk, &irqs)) {
rlim_cur = proc->tsk->signal->rlim[RLIMIT_NOFILE].rlim_cur;
unlock_task_sighand(proc->tsk, &irqs);
}
if (fd >= rlim_cur)
goto out;
Expand Down Expand Up @@ -371,17 +375,16 @@ int task_get_unused_fd_flags(struct task_struct *tsk, int flags)

out:
spin_unlock(&files->file_lock);
put_files_struct(files);
return error;
}

/*
* copied from fd_install
*/
static void task_fd_install(
struct task_struct *tsk, unsigned int fd, struct file *file)
struct binder_proc *proc, unsigned int fd, struct file *file)
{
struct files_struct *files = get_files_struct(tsk);
struct files_struct *files = proc->files;
struct fdtable *fdt;

if (files == NULL)
Expand All @@ -392,7 +395,6 @@ static void task_fd_install(
BUG_ON(fdt->fd[fd] != NULL);
rcu_assign_pointer(fdt->fd[fd], file);
spin_unlock(&files->file_lock);
put_files_struct(files);
}

/*
Expand All @@ -409,10 +411,10 @@ static void __put_unused_fd(struct files_struct *files, unsigned int fd)
/*
* copied from sys_close
*/
static long task_close_fd(struct task_struct *tsk, unsigned int fd)
static long task_close_fd(struct binder_proc *proc, unsigned int fd)
{
struct file *filp;
struct files_struct *files = get_files_struct(tsk);
struct files_struct *files = proc->files;
struct fdtable *fdt;
int retval;

Expand All @@ -439,12 +441,10 @@ static long task_close_fd(struct task_struct *tsk, unsigned int fd)
retval == -ERESTART_RESTARTBLOCK))
retval = -EINTR;

put_files_struct(files);
return retval;

out_unlock:
spin_unlock(&files->file_lock);
put_files_struct(files);
return -EBADF;
}

Expand Down Expand Up @@ -1549,13 +1549,13 @@ binder_transaction(struct binder_proc *proc, struct binder_thread *thread,
return_error = BR_FAILED_REPLY;
goto err_fget_failed;
}
target_fd = task_get_unused_fd_flags(target_proc->tsk, O_CLOEXEC);
target_fd = task_get_unused_fd_flags(target_proc, O_CLOEXEC);
if (target_fd < 0) {
fput(file);
return_error = BR_FAILED_REPLY;
goto err_get_unused_fd_failed;
}
task_fd_install(target_proc->tsk, target_fd, file);
task_fd_install(target_proc, target_fd, file);
if (binder_debug_mask & BINDER_DEBUG_TRANSACTION)
printk(KERN_INFO " fd %ld -> %d\n", fp->handle, target_fd);
/* TODO: fput? */
Expand Down Expand Up @@ -1698,7 +1698,7 @@ binder_transaction_buffer_release(struct binder_proc *proc, struct binder_buffer
if (binder_debug_mask & BINDER_DEBUG_TRANSACTION)
printk(KERN_INFO " fd %ld\n", fp->handle);
if (failed_at)
task_close_fd(proc->tsk, fp->handle);
task_close_fd(proc, fp->handle);
break;

default:
Expand Down Expand Up @@ -2663,6 +2663,34 @@ static void binder_vma_open(struct vm_area_struct *vma)
(unsigned long)pgprot_val(vma->vm_page_prot));
dump_stack();
}

static void binder_release_files(struct work_struct *work)
{
struct binder_proc *proc;
struct files_struct *files;
do {
mutex_lock(&binder_lock);
mutex_lock(&binder_release_files_lock);
if (!hlist_empty(&binder_release_files_list)) {
proc = hlist_entry(binder_release_files_list.first,
struct binder_proc, release_files_node);
hlist_del_init(&proc->release_files_node);
files = proc->files;
if (files)
proc->files = NULL;
} else {
proc = NULL;
files = NULL;
}
mutex_unlock(&binder_release_files_lock);
mutex_unlock(&binder_lock);
if (files)
put_files_struct(files);
} while (proc);
}

static DECLARE_WORK(binder_release_files_work, binder_release_files);

static void binder_vma_close(struct vm_area_struct *vma)
{
struct binder_proc *proc = vma->vm_private_data;
Expand All @@ -2673,6 +2701,13 @@ static void binder_vma_close(struct vm_area_struct *vma)
(vma->vm_end - vma->vm_start) / SZ_1K, vma->vm_flags,
(unsigned long)pgprot_val(vma->vm_page_prot));
proc->vma = NULL;
mutex_lock(&binder_release_files_lock);
if (proc->files) {
hlist_add_head(&proc->release_files_node,
&binder_release_files_list);
schedule_work(&binder_release_files_work);
}
mutex_unlock(&binder_release_files_lock);
}

static struct vm_operations_struct binder_vm_ops = {
Expand Down Expand Up @@ -2751,6 +2786,7 @@ static int binder_mmap(struct file *filp, struct vm_area_struct *vma)
binder_insert_free_buffer(proc, buffer);
proc->free_async_space = proc->buffer_size / 2;
barrier();
proc->files = get_files_struct(current);
proc->vma = vma;

/*printk(KERN_INFO "binder_mmap: %d %lx-%lx maps %p\n", proc->pid, vma->vm_start, vma->vm_end, proc->buffer);*/
Expand Down Expand Up @@ -2831,6 +2867,7 @@ static int binder_release(struct inode *nodp, struct file *filp)
struct hlist_node *pos;
struct binder_transaction *t;
struct rb_node *n;
struct files_struct *files;
struct binder_proc *proc = filp->private_data;
int threads, nodes, incoming_refs, outgoing_refs, buffers, active_transactions, page_count;

Expand All @@ -2840,6 +2877,14 @@ static int binder_release(struct inode *nodp, struct file *filp)
remove_proc_entry(strbuf, binder_proc_dir_entry_proc);
}
mutex_lock(&binder_lock);
mutex_lock(&binder_release_files_lock);
if (!hlist_unhashed(&proc->release_files_node))
hlist_del(&proc->release_files_node);
files = proc->files;
if (files)
proc->files = NULL;
mutex_unlock(&binder_release_files_lock);

hlist_del(&proc->proc_node);
if (binder_context_mgr_node && binder_context_mgr_node->proc == proc) {
if (binder_debug_mask & BINDER_DEBUG_DEAD_BINDER)
Expand Down Expand Up @@ -2937,6 +2982,8 @@ static int binder_release(struct inode *nodp, struct file *filp)
proc->pid, threads, nodes, incoming_refs, outgoing_refs, active_transactions, buffers, page_count);

kfree(proc);
if (files)
put_files_struct(files);
return 0;
}

Expand Down

0 comments on commit 282ca17

Please sign in to comment.