Hao Xu wrote on Tue, Jul 11, 2023 at 07:40:27PM +0800:
> diff --git a/io_uring/fs.c b/io_uring/fs.c
> index f6a69a549fd4..77f00577e09c 100644
> --- a/io_uring/fs.c
> +++ b/io_uring/fs.c
> @@ -291,3 +298,56 @@ void io_link_cleanup(struct io_kiocb *req)
> putname(sl->oldpath);
> putname(sl->newpath);
> }
> +
> +int io_getdents_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
> +{
> + struct io_getdents *gd = io_kiocb_to_cmd(req, struct io_getdents);
> +
> + if (READ_ONCE(sqe->off) != 0)
> + return -EINVAL;
> +
> + gd->dirent = u64_to_user_ptr(READ_ONCE(sqe->addr));
> + gd->count = READ_ONCE(sqe->len);
> +
> + return 0;
> +}
> +
> +int io_getdents(struct io_kiocb *req, unsigned int issue_flags)
> +{
> + struct io_getdents *gd = io_kiocb_to_cmd(req, struct io_getdents);
> + struct file *file;
> + unsigned long getdents_flags = 0;
> + bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
> + bool should_lock = false;
> + int ret;
> +
> + if (force_nonblock) {
> + if (!(req->file->f_mode & FMODE_NOWAIT))
> + return -EAGAIN;
> +
> + getdents_flags = DIR_CONTEXT_F_NOWAIT;
> + }
> +
> + file = req->file;
> + if (file && (file->f_mode & FMODE_ATOMIC_POS)) {
If file is NULL here things will just blow up in vfs_getdents anyway,
let's remove the useless check
> + if (file_count(file) > 1)
I was curious about this so I found it's basically what __fdget_pos does
before deciding it should take the f_pos_lock, and as such this is
probably correct... But if someone can chime in here: what guarantees
someone else won't __fdget_pos (or equivalent through this) the file
again between this and the vfs_getdents call?
That second get would make file_count > 1 and it would lock, but lock
hadn't been taken here so the other call could get the lock without
waiting and both would process getdents or seek or whatever in
parallel.
That aside I don't see any obvious problem with this.
--
Dominique Martinet | Asmadeus
