[PATCH] io_uring/register: use correct location for io_rings_layout

[PATCH] io_uring/register: use correct location for io_rings_layout

[Jens Axboe]

A previous consolidated the ring size etc calculations into
io_prepare_config(), but missed updating io_register_resize_rings()
correctly to use the calculated values. As a result, it ended up using
on-stack uninitialized values, and hence either failed validating the
size correctly, or just failed resizing because the sizes were random.

This caused failures in the liburing regression tests:

[...]
Running test resize-rings.t
resize=-7
test_basic 3000 failed
Test resize-rings.t failed with ret 1
Running test resize-rings.t /dev/sda
resize=-7
test_basic 3000 failed
Test resize-rings.t failed with ret 1
Running test resize-rings.t /dev/nvme1n1
resize=-7
test_basic 3000 failed
Test resize-rings.t failed with ret 1
Running test resize-rings.t /dev/dm-0
resize=-7
test_basic 3000 failed
Test resize-rings.t failed with ret 1

because io_create_region() would return -E2BIG because of unintialized
reg->size values.

Adjust the struct io_rings_layout rl pointer to point to the correct
location, and remove the (now dead) __rl on stack struct.

Fixes: eb76ff6a6829 ("io_uring: pre-calculate scq layout")
Signed-off-by: Jens Axboe <axboe@kernel.dk>

---

diff --git a/io_uring/register.c b/io_uring/register.c
index fc66a5364483..db42f98562c4 100644
--- a/io_uring/register.c
+++ b/io_uring/register.c
@@ -403,7 +403,7 @@ static int io_register_resize_rings(struct io_ring_ctx *ctx, void __user *arg)
 	struct io_ring_ctx_rings o = { }, n = { }, *to_free = NULL;
 	unsigned i, tail, old_head;
 	struct io_uring_params *p = &config.p;
-	struct io_rings_layout __rl, *rl = &__rl;
+	struct io_rings_layout *rl = &config.layout;
 	int ret;
 
 	memset(&config, 0, sizeof(config));

-- 
Jens Axboe

Re: [PATCH] io_uring/register: use correct location for io_rings_layout

[Pavel Begunkov]

On 11/19/25 02:36, Jens Axboe wrote:
> A previous consolidated the ring size etc calculations into
> io_prepare_config(), but missed updating io_register_resize_rings()
> correctly to use the calculated values. As a result, it ended up using
> on-stack uninitialized values, and hence either failed validating the
> size correctly, or just failed resizing because the sizes were random.
> 
> This caused failures in the liburing regression tests:

That made me wonder how it could possibly pass tests for me. I even
made sure it was reaching the final return. Turns out the layout was
0 initialised, region creation fails with -EINVAL, and then the
resizing test just silently skips sub-cases. It'd be great to have
a "not supported, skip" message.

-- 
Pavel Begunkov

Re: [PATCH] io_uring/register: use correct location for io_rings_layout

[Jens Axboe]

On 11/19/25 10:18 AM, Pavel Begunkov wrote:
> On 11/19/25 02:36, Jens Axboe wrote:
>> A previous consolidated the ring size etc calculations into
>> io_prepare_config(), but missed updating io_register_resize_rings()
>> correctly to use the calculated values. As a result, it ended up using
>> on-stack uninitialized values, and hence either failed validating the
>> size correctly, or just failed resizing because the sizes were random.
>>
>> This caused failures in the liburing regression tests:
> 
> That made me wonder how it could possibly pass tests for me. I even
> made sure it was reaching the final return. Turns out the layout was
> 0 initialised, region creation fails with -EINVAL, and then the
> resizing test just silently skips sub-cases. It'd be great to have
> a "not supported, skip" message.

Looks like the test runs into -EINVAL, then tries the DEFER case,
and then doesn't check for SKIP for that. And then it returns
success. I've added a commit for that now, so it'll return 77/SKIP
if it does skip.

I try to avoid having tests be verbose, unless they fail. Otherwise
it's easy to lose information you actually want in the noise. But
it certainly should return T_EXIT_SKIP, when it skips!

-- 
Jens Axboe

Re: [PATCH] io_uring/register: use correct location for io_rings_layout

[Pavel Begunkov]

On 11/19/25 20:22, Jens Axboe wrote:
> On 11/19/25 10:18 AM, Pavel Begunkov wrote:
>> On 11/19/25 02:36, Jens Axboe wrote:
>>> A previous consolidated the ring size etc calculations into
>>> io_prepare_config(), but missed updating io_register_resize_rings()
>>> correctly to use the calculated values. As a result, it ended up using
>>> on-stack uninitialized values, and hence either failed validating the
>>> size correctly, or just failed resizing because the sizes were random.
>>>
>>> This caused failures in the liburing regression tests:
>>
>> That made me wonder how it could possibly pass tests for me. I even
>> made sure it was reaching the final return. Turns out the layout was
>> 0 initialised, region creation fails with -EINVAL, and then the
>> resizing test just silently skips sub-cases. It'd be great to have
>> a "not supported, skip" message.
> 
> Looks like the test runs into -EINVAL, then tries the DEFER case,
> and then doesn't check for SKIP for that. And then it returns
> success. I've added a commit for that now, so it'll return 77/SKIP
> if it does skip.
> 
> I try to avoid having tests be verbose, unless they fail. Otherwise
> it's easy to lose information you actually want in the noise. But
> it certainly should return T_EXIT_SKIP, when it skips!

Printing when tests are skipped was pretty useful because I
expect a latest kernel (+configured for testing setup) to be
to run all tests, and I'd find "test skipped" suspicious by
default. Certainly a test infra problem, but at least it
worked.

At some point it might be great to distinguish when it skips
because of unsupported io_uring features from when some
resources are not available.

On the topic, I've found this in the runner:

elif [ "${#SKIPPED[*]}" -ne 0 ] && [ -n "$TEST_GNU_EXITCODE" ]; then
	exit 77
else
	echo "All tests passed"
	exit 0
fi

But not sure who would even define TEST_GNU_EXITCODE. It should
be more helpful to always print skipped tests:

else
	echo "Tests: skipped $SKIPPED"
	echo "All tests passed"
	exit 0
fi

-- 
Pavel Begunkov

Re: [PATCH] io_uring/register: use correct location for io_rings_layout

[Jens Axboe]

On 11/21/25 6:49 AM, Pavel Begunkov wrote:
> On 11/19/25 20:22, Jens Axboe wrote:
>> On 11/19/25 10:18 AM, Pavel Begunkov wrote:
>>> On 11/19/25 02:36, Jens Axboe wrote:
>>>> A previous consolidated the ring size etc calculations into
>>>> io_prepare_config(), but missed updating io_register_resize_rings()
>>>> correctly to use the calculated values. As a result, it ended up using
>>>> on-stack uninitialized values, and hence either failed validating the
>>>> size correctly, or just failed resizing because the sizes were random.
>>>>
>>>> This caused failures in the liburing regression tests:
>>>
>>> That made me wonder how it could possibly pass tests for me. I even
>>> made sure it was reaching the final return. Turns out the layout was
>>> 0 initialised, region creation fails with -EINVAL, and then the
>>> resizing test just silently skips sub-cases. It'd be great to have
>>> a "not supported, skip" message.
>>
>> Looks like the test runs into -EINVAL, then tries the DEFER case,
>> and then doesn't check for SKIP for that. And then it returns
>> success. I've added a commit for that now, so it'll return 77/SKIP
>> if it does skip.
>>
>> I try to avoid having tests be verbose, unless they fail. Otherwise
>> it's easy to lose information you actually want in the noise. But
>> it certainly should return T_EXIT_SKIP, when it skips!
> 
> Printing when tests are skipped was pretty useful because I
> expect a latest kernel (+configured for testing setup) to be
> to run all tests, and I'd find "test skipped" suspicious by
> default. Certainly a test infra problem, but at least it
> worked.

Depends on why they are skipped - lots of tests skip because they are
given an argument, and they don't support using an argument. When I run
tests I use a bunch of different files/devices, and then you get a lot
of:

Running test pipe.t                                                 0 sec [0]
Running test pipe.t /dev/sda                                        Skipped
Running test pipe.t /dev/nvme1n1                                    Skipped
Running test pipe.t /dev/dm-0                                       Skipped
[...]

for example. And those skips are not interesting at all. The ones that
skip because the kernel doesn't support them, those would be interesting
to dump at the bottom for a better overview.

> At some point it might be great to distinguish when it skips
> because of unsupported io_uring features from when some
> resources are not available.
> 
> On the topic, I've found this in the runner:
> 
> elif [ "${#SKIPPED[*]}" -ne 0 ] && [ -n "$TEST_GNU_EXITCODE" ]; then
>     exit 77
> else
>     echo "All tests passed"
>     exit 0
> fi
> 
> But not sure who would even define TEST_GNU_EXITCODE. It should
> be more helpful to always print skipped tests:
> 
> else
>     echo "Tests: skipped $SKIPPED"
>     echo "All tests passed"
>     exit 0
> fi

Yeah no idea who uses TEST_GNU_EXITCODE... I strongly suspect no one,
and we can just assume 77.

-- 
Jens Axboe