[PATCH 0/2] two small patches

[PATCH 0/2] two small patches

[Pavel Begunkov]

Small patches improving userspace values handling.

Pavel Begunkov (2):
  io_uring: fix overflows checks in provide buffers
  io_uring: check register restriction afore quiesce

 fs/io_uring.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

-- 
2.24.0

[PATCH 1/2] io_uring: fix overflows checks in provide buffers

[Pavel Begunkov]

Colin reported before possible overflow and sign extension problems in
io_provide_buffers_prep(). As Linus pointed out previous attempt did nothing
useful, see d81269fecb8ce ("io_uring: fix provide_buffers sign extension").

Do that with help of check_<op>_overflow helpers. And fix struct
io_provide_buf::len type, as it doesn't make much sense to keep it
signed.

Reported-by: Colin Ian King <colin.king@canonical.com>
Fixes: efe68c1ca8f49 ("io_uring: validate the full range of provided buffers for access")
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 fs/io_uring.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index e9d60dee075e..b57994443b2c 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -627,7 +627,7 @@ struct io_splice {
 struct io_provide_buf {
 	struct file			*file;
 	__u64				addr;
-	__s32				len;
+	__u32				len;
 	__u32				bgid;
 	__u16				nbufs;
 	__u16				bid;
@@ -3923,7 +3923,7 @@ static int io_remove_buffers(struct io_kiocb *req, unsigned int issue_flags)
 static int io_provide_buffers_prep(struct io_kiocb *req,
 				   const struct io_uring_sqe *sqe)
 {
-	unsigned long size;
+	unsigned long size, tmp_check;
 	struct io_provide_buf *p = &req->pbuf;
 	u64 tmp;
 
@@ -3937,6 +3937,12 @@ static int io_provide_buffers_prep(struct io_kiocb *req,
 	p->addr = READ_ONCE(sqe->addr);
 	p->len = READ_ONCE(sqe->len);
 
+	if (check_mul_overflow((unsigned long)p->len, (unsigned long)p->nbufs,
+				&size))
+		return -EOVERFLOW;
+	if (check_add_overflow((unsigned long)p->addr, size, &tmp_check))
+		return -EOVERFLOW;
+
 	size = (unsigned long)p->len * p->nbufs;
 	if (!access_ok(u64_to_user_ptr(p->addr), size))
 		return -EFAULT;
-- 
2.24.0

[PATCH 2/2] io_uring: check register restriction afore quiesce

[Pavel Begunkov]

Move restriction checks of __io_uring_register() before quiesce, saves
from waiting for requests in fail case and simplifies the code a bit.
Also add array_index_nospec() for safety

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 fs/io_uring.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index b57994443b2c..357993e3e0d2 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -9764,6 +9764,14 @@ static int __io_uring_register(struct io_ring_ctx *ctx, unsigned opcode,
 	if (percpu_ref_is_dying(&ctx->refs))
 		return -ENXIO;
 
+	if (ctx->restricted) {
+		if (opcode >= IORING_REGISTER_LAST)
+			return -EINVAL;
+		opcode = array_index_nospec(opcode, IORING_REGISTER_LAST);
+		if (!test_bit(opcode, ctx->restrictions.register_op))
+			return -EACCES;
+	}
+
 	if (io_register_op_must_quiesce(opcode)) {
 		percpu_ref_kill(&ctx->refs);
 
@@ -9792,18 +9800,6 @@ static int __io_uring_register(struct io_ring_ctx *ctx, unsigned opcode,
 		}
 	}
 
-	if (ctx->restricted) {
-		if (opcode >= IORING_REGISTER_LAST) {
-			ret = -EINVAL;
-			goto out;
-		}
-
-		if (!test_bit(opcode, ctx->restrictions.register_op)) {
-			ret = -EACCES;
-			goto out;
-		}
-	}
-
 	switch (opcode) {
 	case IORING_REGISTER_BUFFERS:
 		ret = io_sqe_buffers_register(ctx, arg, nr_args);
@@ -9877,7 +9873,6 @@ static int __io_uring_register(struct io_ring_ctx *ctx, unsigned opcode,
 		break;
 	}
 
-out:
 	if (io_register_op_must_quiesce(opcode)) {
 		/* bring the ctx back to life */
 		percpu_ref_reinit(&ctx->refs);
-- 
2.24.0

Re: [PATCH 0/2] two small patches

[Pavel Begunkov]

On 15/04/2021 13:07, Pavel Begunkov wrote:
> Small patches improving userspace values handling.

imho, those are good for 5.13, as well as look small and clean

> 
> Pavel Begunkov (2):
>   io_uring: fix overflows checks in provide buffers
>   io_uring: check register restriction afore quiesce
> 
>  fs/io_uring.c | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
> 

-- 
Pavel Begunkov

Re: [PATCH 0/2] two small patches

[Jens Axboe]

On 4/15/21 6:07 AM, Pavel Begunkov wrote:
> Small patches improving userspace values handling.
> 
> Pavel Begunkov (2):
>   io_uring: fix overflows checks in provide buffers
>   io_uring: check register restriction afore quiesce
> 
>  fs/io_uring.c | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)

Applied, thanks.

-- 
Jens Axboe

Re: [PATCH 0/2] two small patches

[Jens Axboe]

On 8/4/21 12:29 PM, Vincent Fu wrote:
> Hello Jens,
> 
> Please consider these two small patches that (1) clarify an error
> message and (2) fix a segfault.
> 
> Vincent
> 
> Vincent Fu (2):
>   backend: clarify io scheduler setting error message
>   ioengines: fix crash with --enghelp option
> 
>  backend.c   |  2 +-
>  ioengines.c | 10 +++++-----
>  2 files changed, 6 insertions(+), 6 deletions(-)

Applied, thanks.

-- 
Jens Axboe

[PATCH 0/2] two small patches

[Vincent Fu]

Hello Jens,

Please consider these two small patches that (1) clarify an error
message and (2) fix a segfault.

Vincent

Vincent Fu (2):
  backend: clarify io scheduler setting error message
  ioengines: fix crash with --enghelp option

 backend.c   |  2 +-
 ioengines.c | 10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

-- 
2.25.1