Re: [PATCH for-next] io_uring/query: check for loops in in_query()

[Pavel Begunkov <asml.silence@gmail.com>]

On 9/11/25 10:02, Pavel Begunkov wrote:
> On 9/11/25 01:13, Jens Axboe wrote:
>> io_query() loops over query items that the application or liburing
>> passes in. But it has no checking for a max number of items, or if a
>> loop could be present. If someone were to do:
>>
>>          struct io_uring_query_hdr hdr1, hdr2, hdr3;
>>
>>          hdr3.next_entry = &hdr1;
>>          hdr2.next_entry = &hdr3;
>>          hdr1.next_entry = &hdr2;
>>
>>          io_uring_register(fd, IORING_REGISTER_QUERY, &hdr1, 0);
>>
>> then it'll happily loop forever and process hdr1 -> hdr2 -> hdr3 and
>> then loop back to hdr1.
>>
>> Add a max cap for these kinds of cases, which is arbitrarily set to
>> 1024 as well. Since there's now a cap, it seems that it would be saner
>> to have this interface return the number of items processed. Eg 0..N
>> for success, and < 0 for an error. Then if someone does need to query
>> more than the supported number of items, they can do so iteratively.
> 
> That worsens usability. The user would have to know / count how
> many entries there was in the first place, retry, and do all
> handling. It'll be better to:
> 
> if (nr > (1U << 20))
>      return -ERANGE;
> if (fatal_signal_pending())
>      return -EINTR;
> ...
> return 0;
> 
> 
> 1M should be high enough for future proofing and to protect from
> mildly insane users (and would still be fast enough). I also had
> cond_resched() in some version, but apparently it got lost as
> well.

Tested the diff below, works well enough. 1M breaks out after a
second even in a very underpowered VM.


commit 49f9c37c612967f9a6111e113c072aace68099bf
Author: Pavel Begunkov <asml.silence@gmail.com>
Date:   Thu Sep 11 12:28:18 2025 +0100

     io_uring/query: prevent infinite loops
     
     If the query chain forms a cycle, the interface will loop indefinitely.
     Cap the number of iterations at 1M, that should hopefully be enough for
     even the most inventive future use of the interface. Also make sure it
     checks for fatal signals and reschedules when necessary.
     
     Fixes: c265ae75f900 ("io_uring: introduce io_uring querying")
     Reported-by: Jens Axboe <axboe@kernel.dk>
     Suggested-by: Jens Axboe <axboe@kernel.dk>
     Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>

diff --git a/io_uring/query.c b/io_uring/query.c
index 9eed0f371956..1af40e5e9a2a 100644
--- a/io_uring/query.c
+++ b/io_uring/query.c
@@ -6,6 +6,7 @@
  #include "io_uring.h"
  
  #define IO_MAX_QUERY_SIZE		(sizeof(struct io_uring_query_opcode))
+#define IO_MAX_QUERY_ENTRIES		(1U << 20)
  
  static ssize_t io_query_ops(void *data)
  {
@@ -74,7 +75,7 @@ int io_query(struct io_ring_ctx *ctx, void __user *arg, unsigned nr_args)
  {
  	char entry_buffer[IO_MAX_QUERY_SIZE];
  	void __user *uhdr = arg;
-	int ret;
+	int ret, nr = 0;
  
  	memset(entry_buffer, 0, sizeof(entry_buffer));
  
@@ -88,6 +89,13 @@ int io_query(struct io_ring_ctx *ctx, void __user *arg, unsigned nr_args)
  		if (ret)
  			return ret;
  		uhdr = u64_to_user_ptr(next_hdr);
+
+		/* Have some limit to avoid a potential cycle */
+		if (++nr >= IO_MAX_QUERY_ENTRIES)
+			return -ERANGE;
+		if (fatal_signal_pending(current))
+			return -EINTR;
+		cond_resched();
  	}
  	return 0;
  }