From 80cd795630d6526ba729a089a435bf74a57af927 Mon Sep 17 00:00:00 2001 From: Todd Kjos Date: Fri, 14 Dec 2018 15:58:21 -0800 Subject: binder: fix use-after-free due to ksys_close() during fdget() 44d8047f1d8 ("binder: use standard functions to allocate fds") exposed a pre-existing issue in the binder driver. fdget() is used in ksys_ioctl() as a performance optimization. One of the rules associated with fdget() is that ksys_close() must not be called between the fdget() and the fdput(). There is a case where this requirement is not met in the binder driver which results in the reference count dropping to 0 when the device is still in use. This can result in use-after-free or other issues. If userpace has passed a file-descriptor for the binder driver using a BINDER_TYPE_FDA object, then kys_close() is called on it when handling a binder_ioctl(BC_FREE_BUFFER) command. This violates the assumptions for using fdget(). The problem is fixed by deferring the close using task_work_add(). A new variant of __close_fd() was created that returns a struct file with a reference. The fput() is deferred instead of using ksys_close(). Fixes: 44d8047f1d87a ("binder: use standard functions to allocate fds") Suggested-by: Al Viro Signed-off-by: Todd Kjos Cc: stable Signed-off-by: Greg Kroah-Hartman --- fs/file.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) (limited to 'fs') diff --git a/fs/file.c b/fs/file.c index 7ffd6e9d103d..8d059d8973e9 100644 --- a/fs/file.c +++ b/fs/file.c @@ -640,6 +640,35 @@ out_unlock: } EXPORT_SYMBOL(__close_fd); /* for ksys_close() */ +/* + * variant of __close_fd that gets a ref on the file for later fput + */ +int __close_fd_get_file(unsigned int fd, struct file **res) +{ + struct files_struct *files = current->files; + struct file *file; + struct fdtable *fdt; + + spin_lock(&files->file_lock); + fdt = files_fdtable(files); + if (fd >= fdt->max_fds) + goto out_unlock; + file = fdt->fd[fd]; + if (!file) + goto out_unlock; + rcu_assign_pointer(fdt->fd[fd], NULL); + __put_unused_fd(files, fd); + spin_unlock(&files->file_lock); + get_file(file); + *res = file; + return filp_close(file, files); + +out_unlock: + spin_unlock(&files->file_lock); + *res = NULL; + return -ENOENT; +} + void do_close_on_exec(struct files_struct *files) { unsigned i; -- cgit v1.2.3