[PATCH 0/4] io_uring: verify that reserved fields are 0

[PATCH 0/4] io_uring: verify that reserved fields are 0

[Dylan Yudaken]

A few reserved fields are not verified to be 0. In preparation for possibly using these fields later we should verify that they are passed as 0.

One extra field I do not have confidence in verifying is up.nr in io_register_files_update(). Should this also be checked to be zero?

Patch 1 in this series just moves a validation out of __io_register_rsrc_update as it was duplicated
Patch 2-4 add verifications for reserved fields

Dylan Yudaken (4):
  io_uring: move io_uring_rsrc_update2 validation
  io_uring: verify that resv2 is 0 in io_uring_rsrc_update2
  io_uring: verify resv is 0 in ringfd register/unregister
  io_uring: verify pad field is 0 in io_get_ext_arg

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


base-commit: 0f8da75b51ac863b9435368bd50691718cc454b0
-- 
2.30.2

[PATCH 1/4] io_uring: move io_uring_rsrc_update2 validation

[Dylan Yudaken]

Move validation to be more consistently straight after
copy_from_user. This is already done in io_register_rsrc_update and so
this removes that redundant check.

Signed-off-by: Dylan Yudaken <dylany@fb.com>
---
 fs/io_uring.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 8a931eb8a3a6..58bfa71fe3b6 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -11398,8 +11398,6 @@ static int __io_register_rsrc_update(struct io_ring_ctx *ctx, unsigned type,
 	__u32 tmp;
 	int err;
 
-	if (up->resv)
-		return -EINVAL;
 	if (check_add_overflow(up->offset, nr_args, &tmp))
 		return -EOVERFLOW;
 	err = io_rsrc_node_switch_start(ctx);
@@ -11425,6 +11423,8 @@ static int io_register_files_update(struct io_ring_ctx *ctx, void __user *arg,
 	memset(&up, 0, sizeof(up));
 	if (copy_from_user(&up, arg, sizeof(struct io_uring_rsrc_update)))
 		return -EFAULT;
+	if (up.resv)
+		return -EINVAL;
 	return __io_register_rsrc_update(ctx, IORING_RSRC_FILE, &up, nr_args);
 }
 
-- 
2.30.2

[PATCH 2/4] io_uring: verify that resv2 is 0 in io_uring_rsrc_update2

[Dylan Yudaken]

Verify that the user does not pass in anything but 0 for this field.

Fixes: 992da01aa932 ("io_uring: change registration/upd/rsrc tagging ABI")
Signed-off-by: Dylan Yudaken <dylany@fb.com>
---
 fs/io_uring.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 58bfa71fe3b6..e899192ffb77 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -6839,6 +6839,7 @@ static int io_files_update(struct io_kiocb *req, unsigned int issue_flags)
 	up.nr = 0;
 	up.tags = 0;
 	up.resv = 0;
+	up.resv2 = 0;
 
 	io_ring_submit_lock(ctx, needs_lock);
 	ret = __io_register_rsrc_update(ctx, IORING_RSRC_FILE,
@@ -11423,7 +11424,7 @@ static int io_register_files_update(struct io_ring_ctx *ctx, void __user *arg,
 	memset(&up, 0, sizeof(up));
 	if (copy_from_user(&up, arg, sizeof(struct io_uring_rsrc_update)))
 		return -EFAULT;
-	if (up.resv)
+	if (up.resv || up.resv2)
 		return -EINVAL;
 	return __io_register_rsrc_update(ctx, IORING_RSRC_FILE, &up, nr_args);
 }
@@ -11437,7 +11438,7 @@ static int io_register_rsrc_update(struct io_ring_ctx *ctx, void __user *arg,
 		return -EINVAL;
 	if (copy_from_user(&up, arg, sizeof(up)))
 		return -EFAULT;
-	if (!up.nr || up.resv)
+	if (!up.nr || up.resv || up.resv2)
 		return -EINVAL;
 	return __io_register_rsrc_update(ctx, type, &up, up.nr);
 }
-- 
2.30.2

[PATCH 3/4] io_uring: verify resv is 0 in ringfd register/unregister

[Dylan Yudaken]

Only allow resv field to be 0 in struct io_uring_rsrc_update user
arguments.

Fixes: e7a6c00dc77a ("io_uring: add support for registering ring file descriptors")
Signed-off-by: Dylan Yudaken <dylany@fb.com>
---
 fs/io_uring.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index e899192ffb77..a84bfec97d0d 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -10533,6 +10533,11 @@ static int io_ringfd_register(struct io_ring_ctx *ctx, void __user *__arg,
 			break;
 		}
 
+		if (reg.resv) {
+			ret = -EINVAL;
+			break;
+		}
+
 		if (reg.offset == -1U) {
 			start = 0;
 			end = IO_RINGFD_REG_MAX;
@@ -10579,7 +10584,7 @@ static int io_ringfd_unregister(struct io_ring_ctx *ctx, void __user *__arg,
 			ret = -EFAULT;
 			break;
 		}
-		if (reg.offset >= IO_RINGFD_REG_MAX) {
+		if (reg.resv || reg.offset >= IO_RINGFD_REG_MAX) {
 			ret = -EINVAL;
 			break;
 		}
-- 
2.30.2

[PATCH 4/4] io_uring: verify pad field is 0 in io_get_ext_arg

[Dylan Yudaken]

Ensure that only 0 is passed for pad here.

Fixes: c73ebb685fb6 ("io_uring: add timeout support for io_uring_enter()")
Signed-off-by: Dylan Yudaken <dylany@fb.com>
---
 fs/io_uring.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index a84bfec97d0d..6b1a98697dcf 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -10711,6 +10711,8 @@ static int io_get_ext_arg(unsigned flags, const void __user *argp, size_t *argsz
 		return -EINVAL;
 	if (copy_from_user(&arg, argp, sizeof(arg)))
 		return -EFAULT;
+	if (arg.pad)
+		return -EINVAL;
 	*sig = u64_to_user_ptr(arg.sigmask);
 	*argsz = arg.sigmask_sz;
 	*ts = u64_to_user_ptr(arg.ts);
-- 
2.30.2

Re: [PATCH 0/4] io_uring: verify that reserved fields are 0

[Jens Axboe]

On Tue, 12 Apr 2022 09:30:38 -0700, Dylan Yudaken wrote:
> A few reserved fields are not verified to be 0. In preparation for possibly using these fields later we should verify that they are passed as 0.
> 
> One extra field I do not have confidence in verifying is up.nr in io_register_files_update(). Should this also be checked to be zero?
> 
> Patch 1 in this series just moves a validation out of __io_register_rsrc_update as it was duplicated
> Patch 2-4 add verifications for reserved fields
> 
> [...]

Applied, thanks!

[1/4] io_uring: move io_uring_rsrc_update2 validation
      commit: 565c5e616e8061b40a2e1d786c418a7ac3503a8d
[2/4] io_uring: verify that resv2 is 0 in io_uring_rsrc_update2
      commit: d8a3ba9c143bf89c032deced8a686ffa53b46098
[3/4] io_uring: verify resv is 0 in ringfd register/unregister
      commit: 6fb53cf8ff2c4713247df523404d24f466b98f52
[4/4] io_uring: verify pad field is 0 in io_get_ext_arg
      commit: d2347b9695dafe5c388a5f9aeb70e27a7a4d29cf

Best regards,
-- 
Jens Axboe