Re: [PATCH] io_uring: make OP_CLOSE consistent direct open

[Jens Axboe <axboe@kernel.dk>]

On 9/24/21 1:04 PM, Pavel Begunkov wrote:
> From recently open/accept are now able to manipulate fixed file table,
> but it's inconsistent that close can't. Close the gap, keep API same as
> with open/accept, i.e. via sqe->file_slot.

I really think we should do this for 5.15 to make the API a bit more
sane from the user point of view, folks definitely expect being able
to use IORING_OP_CLOSE with a fixed file that they got with IORING_OP_OPEN,
for example.

How about this small tweak, basically making it follow the same rules
as other commands that do fixed files:

1) Require IOSQE_FIXED_FILE to be set for a direct close. sqe->file_index
   will be the descriptor to close in that case. If sqe->fd is set, we
   -EINVAL the request.

2) If IOSQE_FIXED_FILE isn't set, it's a normal close. As before, if
   sqe->file_index is set and IOSQE_FIXED_FILE isn't, then we -EINVAL
   the request.

Basically this incremental on top of yours.


diff --git a/fs/io_uring.c b/fs/io_uring.c
index 82f867983bb3..dc6e3699779d 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -4596,12 +4596,12 @@ static int io_close_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	if (sqe->ioprio || sqe->off || sqe->addr || sqe->len ||
 	    sqe->rw_flags || sqe->buf_index)
 		return -EINVAL;
-	if (req->flags & REQ_F_FIXED_FILE)
-		return -EBADF;
 
 	req->close.fd = READ_ONCE(sqe->fd);
 	req->close.file_slot = READ_ONCE(sqe->file_index);
-	if (req->close.file_slot && req->close.fd)
+	if (!(req->flags & REQ_F_FIXED_FILE) && req->close.file_slot)
+		return -EINVAL;
+	else if (req->close.fd)
 		return -EINVAL;
 
 	return 0;
@@ -4615,7 +4615,7 @@ static int io_close(struct io_kiocb *req, unsigned int issue_flags)
 	struct file *file = NULL;
 	int ret = -EBADF;
 
-	if (req->close.file_slot) {
+	if (req->flags & REQ_F_FIXED_FILE) {
 		ret = io_close_fixed(req, issue_flags);
 		goto err;
 	}

-- 
Jens Axboe