Re: [PATCH i-g-t] tests/intel/gem_exec_params: add invalid-offset-multiple-buffers test

Hi Krzysztof,

On 2026-08-12 at 16:43:32 +0200, Krzysztof Niemiec wrote:
> Add coverage for the case where the batch start offset is invalid with
> multiple BOs submitted, which was buggy in the kernel until 4fe2bd195435
> ("drm/i915/gem: Zero-initialize the eb.vma array in i915_gem_do_execbuffer")
> 
> Signed-off-by: Krzysztof Niemiec <[email protected]>
> ---
>  tests/intel/gem_exec_params.c | 48 +++++++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
> 
> diff --git a/tests/intel/gem_exec_params.c b/tests/intel/gem_exec_params.c
> index 3ba4c530b..757115a21 100644
> --- a/tests/intel/gem_exec_params.c
> +++ b/tests/intel/gem_exec_params.c
> @@ -85,6 +85,8 @@
>   *
>   * SUBTEST: invalid-flag
>   *
> + * SUBTEST: invalid-offset-multiple-buffers
> + *
>   * SUBTEST: invalid-ring
>   *
>   * SUBTEST: invalid-ring2
> @@ -404,6 +406,49 @@ static void test_invalid_batch_start(int fd)
>  	gem_close(fd, exec.handle);
>  }
>  
> +/*
> + * this test is checking for a NULL deref in eb_release_vmas()
> + * (see 4fe2bd195435 for kernel fix).
> + *
> + * this is always going to fail with an EINVAL but will trigger a crash
> + * in the kernel on affected systems.
You could be a bit more specific than "this":
"the test is supposed to trigger -EINVAL on __gem_execbuf return,
which will lead to a crash on affected systems."

Otherwise, LGTM:
Reviewed-by: Krzysztof Karas <[email protected]>

> + */
> +
> +static void test_invalid_offset_multiple_buffers(int fd)
> +{
> +	struct drm_i915_gem_exec_object2 exec[2];
> +	struct drm_i915_gem_execbuffer2 execbuf;
> +	const intel_ctx_t *ctx;
> +	uint32_t size = 0x1000;
> +
> +	/*
> +	 * clear out junk from the argument structs, since it makes the ioctl
> +	 * fail due to invalid flags, which is not the failure we want to
> +	 * invoke here.
> +	 */
> +
> +	memset(exec, 0, sizeof(exec));
> +	memset(&execbuf, 0, sizeof(execbuf));
> +
> +	ctx = intel_ctx_create_all_physical(fd);
> +	exec[0].handle = batch_create_size(fd, size);
> +	exec[1].handle = batch_create_size(fd, size);
> +
> +	/* this triggers a fail precisely in eb_add_vma() */
> +	execbuf.buffers_ptr = to_user_pointer(exec);
> +	execbuf.buffer_count = 2;
> +	execbuf.batch_start_offset = size;
> +	execbuf.rsvd2 = ctx->id;
> +
> +	igt_assert_eq(__gem_execbuf(fd, &execbuf), -EINVAL);
> +
> +	gem_sync(fd, exec[0].handle);
> +	gem_sync(fd, exec[1].handle);
> +
> +	gem_close(fd, exec[0].handle);
> +	gem_close(fd, exec[1].handle);
> +}
> +
>  static void test_larger_than_life_batch(int fd)
>  {
>  	const struct intel_execution_engine2 *e;
> @@ -705,6 +750,9 @@ int igt_main()
>  	igt_subtest("larger-than-life-batch")
>  		test_larger_than_life_batch(fd);
>  
> +	igt_subtest("invalid-offset-multiple-buffers")
> +		test_invalid_offset_multiple_buffers(fd);
> +
>  #define DIRT(name) \
>  	igt_subtest(#name "-dirt") { \
>  		execbuf.flags = 0; \
> -- 
> 2.55.0
> 

-- 
Best Regards,
Krzysztof