io_uring: ensure we return -EINVAL on unknown opcode

If we submit an unknown opcode and have fd == -1, io_op_needs_file()
will return true as we default to needing a file. Then when we go and
assign the file, we find the 'fd' invalid and return -EBADF. We really
should be returning -EINVAL for that case, as we normally do for
unsupported opcodes.

Change io_op_needs_file() to have the following return values:

0   - does not need a file
1   - does need a file
< 0 - error value

and use this to pass back the right value for this invalid case.

Signed-off-by: Jens Axboe <axboe@kernel.dk>

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 42de210be631073507159b6d7863953c67d3812f..9b1833fedc5ccc3ca829df39e0dc1cbed6b9999e 100644 (file)
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -3062,7 +3062,12 @@ static void io_wq_submit_work(struct io_wq_work **workptr)
        }
 }
 
-static bool io_op_needs_file(const struct io_uring_sqe *sqe)
+static bool io_req_op_valid(int op)
+{
+       return op >= IORING_OP_NOP && op < IORING_OP_LAST;
+}
+
+static int io_op_needs_file(const struct io_uring_sqe *sqe)
 {
        int op = READ_ONCE(sqe->opcode);
 
@@ -3073,9 +3078,11 @@ static bool io_op_needs_file(const struct io_uring_sqe *sqe)
        case IORING_OP_TIMEOUT_REMOVE:
        case IORING_OP_ASYNC_CANCEL:
        case IORING_OP_LINK_TIMEOUT:
-               return false;
+               return 0;
        default:
-               return true;
+               if (io_req_op_valid(op))
+                       return 1;
+               return -EINVAL;
        }
 }
 
@@ -3092,7 +3099,7 @@ static int io_req_set_file(struct io_submit_state *state, struct io_kiocb *req)
 {
        struct io_ring_ctx *ctx = req->ctx;
        unsigned flags;
-       int fd;
+       int fd, ret;
 
        flags = READ_ONCE(req->sqe->flags);
        fd = READ_ONCE(req->sqe->fd);
@@ -3100,8 +3107,9 @@ static int io_req_set_file(struct io_submit_state *state, struct io_kiocb *req)
        if (flags & IOSQE_IO_DRAIN)
                req->flags |= REQ_F_IO_DRAIN;
 
-       if (!io_op_needs_file(req->sqe))
-               return 0;
+       ret = io_op_needs_file(req->sqe);
+       if (ret <= 0)
+               return ret;
 
        if (flags & IOSQE_FIXED_FILE) {
                if (unlikely(!ctx->file_table ||
@@ -3312,7 +3320,6 @@ static inline void io_queue_link_head(struct io_kiocb *req)
                io_queue_sqe(req);
 }
 
-
 #define SQE_VALID_FLAGS        (IOSQE_FIXED_FILE|IOSQE_IO_DRAIN|IOSQE_IO_LINK| \
                                IOSQE_IO_HARDLINK)
 

diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index ea231366f5fd75c2f8e36c7fd961edd33ce01510..a3300e1b9a0100ab8660d5e127eae12ab13c93c6 100644 (file)
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -58,23 +58,28 @@ struct io_uring_sqe {
 #define IORING_SETUP_SQ_AFF    (1U << 2)       /* sq_thread_cpu is valid */
 #define IORING_SETUP_CQSIZE    (1U << 3)       /* app defines CQ size */
 
-#define IORING_OP_NOP          0
-#define IORING_OP_READV                1
-#define IORING_OP_WRITEV       2
-#define IORING_OP_FSYNC                3
-#define IORING_OP_READ_FIXED   4
-#define IORING_OP_WRITE_FIXED  5
-#define IORING_OP_POLL_ADD     6
-#define IORING_OP_POLL_REMOVE  7
-#define IORING_OP_SYNC_FILE_RANGE      8
-#define IORING_OP_SENDMSG      9
-#define IORING_OP_RECVMSG      10
-#define IORING_OP_TIMEOUT      11
-#define IORING_OP_TIMEOUT_REMOVE       12
-#define IORING_OP_ACCEPT       13
-#define IORING_OP_ASYNC_CANCEL 14
-#define IORING_OP_LINK_TIMEOUT 15
-#define IORING_OP_CONNECT      16
+enum {
+       IORING_OP_NOP,
+       IORING_OP_READV,
+       IORING_OP_WRITEV,
+       IORING_OP_FSYNC,
+       IORING_OP_READ_FIXED,
+       IORING_OP_WRITE_FIXED,
+       IORING_OP_POLL_ADD,
+       IORING_OP_POLL_REMOVE,
+       IORING_OP_SYNC_FILE_RANGE,
+       IORING_OP_SENDMSG,
+       IORING_OP_RECVMSG,
+       IORING_OP_TIMEOUT,
+       IORING_OP_TIMEOUT_REMOVE,
+       IORING_OP_ACCEPT,
+       IORING_OP_ASYNC_CANCEL,
+       IORING_OP_LINK_TIMEOUT,
+       IORING_OP_CONNECT,
+
+       /* this goes last, obviously */
+       IORING_OP_LAST,
+};
 
 /*
  * sqe->fsync_flags