[PATCH 4.19 1/3] file: Rename __close_fd_get_file close_fd_get_file
From: "Eric W. Biederman" <ebiederm@xmission.com> mainline inclusion from mainline-v5.11-rc1 commit 9fe83c43e71cdb8e5b9520bcb98706a2b3c680c8 category: bugfix bugzilla: 188963, https://gitee.com/src-openeuler/kernel/issues/I7GUAN CVE: NA -------------------------------- The function close_fd_get_file is explicitly a variant of __close_fd[1]. Now that __close_fd has been renamed close_fd, rename close_fd_get_file to be consistent with close_fd. When __alloc_fd, __close_fd and __fd_install were introduced the double underscore indicated that the function took a struct files_struct parameter. The function __close_fd_get_file never has so the naming has always been inconsistent. This just cleans things up so there are not any lingering mentions or references __close_fd left in the code. [1] 80cd795630d6 ("binder: fix use-after-free due to ksys_close() during fdget()") Link: https://lkml.kernel.org/r/20201120231441.29911-23-ebiederm@xmission.com Signed-off-by: Eric W. Biederman <ebiederm@xmission.com> Conflicts: drivers/android/binder.c fs/file.c fs/io_uring.c include/linux/fdtable.h Signed-off-by: Li Nan <linan122@huawei.com> Reviewed-by: Yang Erkun <yangerkun@huawei.com> Reviewed-by: Wang Weiyang <wangweiyang2@huawei.com> Signed-off-by: Yongqiang Liu <liuyongqiang13@huawei.com> --- drivers/android/binder.c | 2 +- fs/file.c | 4 ++-- fs/io_uring.c | 2 +- include/linux/fdtable.h | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index 720ee4b97879..d73b0df55967 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -2290,7 +2290,7 @@ static void binder_deferred_fd_close(int fd) if (!twcb) return; init_task_work(&twcb->twork, binder_do_fd_close); - __close_fd_get_file(fd, &twcb->file); + close_fd_get_file(fd, &twcb->file); if (twcb->file) { filp_close(twcb->file, current->files); task_work_add(current, &twcb->twork, true); diff --git a/fs/file.c b/fs/file.c index eb10b7634d76..7ada34a76742 100644 --- a/fs/file.c +++ b/fs/file.c @@ -696,11 +696,11 @@ int __close_fd(struct files_struct *files, unsigned fd) EXPORT_SYMBOL(__close_fd); /* for ksys_close() */ /* - * variant of __close_fd that gets a ref on the file for later fput. + * variant of close_fd that gets a ref on the file for later fput. * The caller must ensure that filp_close() called on the file, and then * an fput(). */ -int __close_fd_get_file(unsigned int fd, struct file **res) +int close_fd_get_file(unsigned int fd, struct file **res) { struct files_struct *files = current->files; struct file *file; diff --git a/fs/io_uring.c b/fs/io_uring.c index 7d7af6a0ef96..8d6c0c46e5a0 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -3885,7 +3885,7 @@ static int io_close(struct io_kiocb *req, bool force_nonblock, /* might be already done during nonblock submission */ if (!close->put_file) { - ret = __close_fd_get_file(close->fd, &close->put_file); + ret = close_fd_get_file(close->fd, &close->put_file); if (ret < 0) return (ret == -ENOENT) ? -EBADF : ret; } diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h index 29044f3390d3..05849a82b330 100644 --- a/include/linux/fdtable.h +++ b/include/linux/fdtable.h @@ -122,7 +122,7 @@ extern void __fd_install(struct files_struct *files, unsigned int fd, struct file *file); extern int __close_fd(struct files_struct *files, unsigned int fd); -extern int __close_fd_get_file(unsigned int fd, struct file **res); +extern int close_fd_get_file(unsigned int fd, struct file **res); extern struct kmem_cache *files_cachep; -- 2.25.1
From: Jens Axboe <axboe@kernel.dk> mainline inclusion from mainline-v5.12-rc1 commit 53dec2ea74f2ef360e8455439be96a780baa6097 category: bugfix bugzilla: 188963, https://gitee.com/src-openeuler/kernel/issues/I7GUAN CVE: NA -------------------------------- Assumes current->files->file_lock is already held on invocation. Helps the caller check the file before removing the fd, if it needs to. Signed-off-by: Jens Axboe <axboe@kernel.dk> Conflicts: fs/file.c fs/internal.h Signed-off-by: Li Nan <linan122@huawei.com> Reviewed-by: Yang Erkun <yangerkun@huawei.com> Reviewed-by: Wang Weiyang <wangweiyang2@huawei.com> Signed-off-by: Yongqiang Liu <liuyongqiang13@huawei.com> --- fs/file.c | 36 +++++++++++++++++++++++++----------- fs/internal.h | 1 + 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/fs/file.c b/fs/file.c index 7ada34a76742..b5df6680301c 100644 --- a/fs/file.c +++ b/fs/file.c @@ -20,6 +20,8 @@ #include <linux/rcupdate.h> #include <linux/filescontrol.h> +#include "internal.h" + unsigned int sysctl_nr_open __read_mostly = 1024*1024; unsigned int sysctl_nr_open_min = BITS_PER_LONG; /* our min() is unusable in constant expressions ;-/ */ @@ -696,36 +698,48 @@ int __close_fd(struct files_struct *files, unsigned fd) EXPORT_SYMBOL(__close_fd); /* for ksys_close() */ /* - * variant of close_fd that gets a ref on the file for later fput. - * The caller must ensure that filp_close() called on the file, and then - * an fput(). + * See close_fd_get_file() below, this variant assumes current->files->file_lock + * is held. */ -int close_fd_get_file(unsigned int fd, struct file **res) +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; + goto out_err; file = fdt->fd[fd]; if (!file) - goto out_unlock; + goto out_err; rcu_assign_pointer(fdt->fd[fd], NULL); __put_unused_fd(files, fd); - spin_unlock(&files->file_lock); get_file(file); *res = file; return 0; - -out_unlock: - spin_unlock(&files->file_lock); +out_err: *res = NULL; return -ENOENT; } +/* + * variant of close_fd that gets a ref on the file for later fput. + * The caller must ensure that filp_close() called on the file, and then + * an fput(). + */ +int close_fd_get_file(unsigned int fd, struct file **res) +{ + struct files_struct *files = current->files; + int ret; + + spin_lock(&files->file_lock); + ret = __close_fd_get_file(fd, res); + spin_unlock(&files->file_lock); + + return ret; +} + void do_close_on_exec(struct files_struct *files) { unsigned i; diff --git a/fs/internal.h b/fs/internal.h index e63939e64439..a17f76e7651e 100644 --- a/fs/internal.h +++ b/fs/internal.h @@ -120,6 +120,7 @@ extern struct file *do_filp_open(int dfd, struct filename *pathname, extern struct file *do_file_open_root(struct dentry *, struct vfsmount *, const char *, const struct open_flags *); extern int build_open_flags(int flags, umode_t mode, struct open_flags *op); +extern int __close_fd_get_file(unsigned int fd, struct file **res); long do_sys_ftruncate(unsigned int fd, loff_t length, int small); long do_faccessat(int dfd, const char __user *filename, int mode); -- 2.25.1
From: Jens Axboe <axboe@kernel.dk> mainline inclusion from mainline-v5.12-rc1 commit 9eac1904d3364254d622bf2c771c4f85cd435fc2 category: bugfix bugzilla: 188963, https://gitee.com/src-openeuler/kernel/issues/I7GUAN CVE: CVE-2023-1295 -------------------------------- We currently split the close into two, in case we have a ->flush op that we can't safely handle from non-blocking context. This requires us to flag the op as uncancelable if we do need to punt it async, and that means special handling for just this op type. Use __close_fd_get_file() and grab the files lock so we can get the file and check if we need to go async in one atomic operation. That gets rid of the need for splitting this into two steps, and hence the need for IO_WQ_WORK_NO_CANCEL. Signed-off-by: Jens Axboe <axboe@kernel.dk> Conflict: fs/io_uring.c Signed-off-by: Li Nan <linan122@huawei.com> Reviewed-by: Yang Erkun <yangerkun@huawei.com> Reviewed-by: Wang Weiyang <wangweiyang2@huawei.com> Signed-off-by: Yongqiang Liu <liuyongqiang13@huawei.com> --- fs/io_uring.c | 66 ++++++++++++++++++++++++++++----------------------- 1 file changed, 36 insertions(+), 30 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 8d6c0c46e5a0..ce60df5e4d91 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -369,7 +369,6 @@ struct io_poll_iocb { struct io_close { struct file *file; - struct file *put_file; int fd; }; @@ -829,8 +828,6 @@ static const struct io_op_def io_op_defs[] = { .needs_fs = 1, }, [IORING_OP_CLOSE] = { - .needs_file = 1, - .needs_file_no_error = 1, .file_table = 1, }, [IORING_OP_FILES_UPDATE] = { @@ -3853,13 +3850,6 @@ static int io_statx(struct io_kiocb *req, bool force_nonblock) static int io_close_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe) { - /* - * If we queue this for async, it must not be cancellable. That would - * leave the 'file' in an undeterminate state, and here need to modify - * io_wq_work.flags, so initialize io_wq_work firstly. - */ - io_req_init_async(req); - if (unlikely(req->ctx->flags & (IORING_SETUP_IOPOLL|IORING_SETUP_SQPOLL))) return -EINVAL; if (sqe->ioprio || sqe->off || sqe->addr || sqe->len || @@ -3869,44 +3859,60 @@ static int io_close_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe) return -EBADF; req->close.fd = READ_ONCE(sqe->fd); - if ((req->file && req->file->f_op == &io_uring_fops) || - req->close.fd == req->ctx->ring_fd) - return -EBADF; - - req->close.put_file = NULL; return 0; } static int io_close(struct io_kiocb *req, bool force_nonblock, struct io_comp_state *cs) { + struct files_struct *files = current->files; struct io_close *close = &req->close; + struct fdtable *fdt; + struct file *file; int ret; - /* might be already done during nonblock submission */ - if (!close->put_file) { - ret = close_fd_get_file(close->fd, &close->put_file); - if (ret < 0) - return (ret == -ENOENT) ? -EBADF : ret; + file = NULL; + ret = -EBADF; + spin_lock(&files->file_lock); + fdt = files_fdtable(files); + if (close->fd >= fdt->max_fds) { + spin_unlock(&files->file_lock); + goto err; + } + file = fdt->fd[close->fd]; + if (!file) { + spin_unlock(&files->file_lock); + goto err; + } + + if (file->f_op == &io_uring_fops) { + spin_unlock(&files->file_lock); + file = NULL; + goto err; } /* if the file has a flush method, be safe and punt to async */ - if (close->put_file->f_op->flush && force_nonblock) { - /* not safe to cancel at this point */ - req->work.flags |= IO_WQ_WORK_NO_CANCEL; - /* was never set, but play safe */ - req->flags &= ~REQ_F_NOWAIT; - /* avoid grabbing files - we don't need the files */ - req->flags |= REQ_F_NO_FILE_TABLE; + if ((file->f_op->flush && force_nonblock) || + req->close.fd == req->ctx->ring_fd) { + spin_unlock(&files->file_lock); return -EAGAIN; } + ret = __close_fd_get_file(close->fd, &file); + spin_unlock(&files->file_lock); + if (ret < 0) { + if (ret == -ENOENT) + ret = -EBADF; + goto err; + } + /* No ->flush() or already async, safely close from here */ - ret = filp_close(close->put_file, req->work.files ? : current->files); + ret = filp_close(file, current->files); +err: if (ret < 0) req_set_fail_links(req); - fput(close->put_file); - close->put_file = NULL; + if (file) + fput(file); __io_req_complete(req, ret, 0, cs); return 0; } -- 2.25.1
participants (1)
-
Yongqiang Liu