[PATCH] drm/i915/gem: Prevent overstepping exec array boundary

[PATCH] drm/i915/gem: Prevent overstepping exec array boundary

[Krzysztof Karas]

It is possible for eb_relocate_parse_slow() to call kvfree() on
entries outside the original exec array during following
scenario inside eb_relocate_parse_slow():

1) eb_copy_relocations() allocates as many as eb->buffer_count
   copies for exec entries;
2) eb_parse() appends up to two VMAs, right after incrementing
   eb->buffer_count;
3) cleanup under "out" label uses this incremented buffer_count
   to release relocation pointers from exec array via kvfree(),
   not from VMA array.

To amend this problem, ensure that relocation cleanup uses the
original exec object count, excluding the VMAs appended by the
command parser.

Fixes: 8e4ba491b0ba ("drm/i915: Parse command buffer earlier in eb_relocate(slow)")
Cc: stable@vger.kernel.org # 5.10+
Assisted-by: GitHub-Copilot:gpt-6-astra
Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 1f303d4eaa4d4..d5aadced7da0c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1766,6 +1766,7 @@ static int eb_reinit_userptr(struct i915_execbuffer *eb)
 
 static noinline int eb_relocate_parse_slow(struct i915_execbuffer *eb)
 {
+	const unsigned int count = eb->buffer_count;
 	bool have_copy = false;
 	struct eb_vma *ev;
 	int err = 0;
@@ -1868,7 +1869,6 @@ static noinline int eb_relocate_parse_slow(struct i915_execbuffer *eb)
 
 out:
 	if (have_copy) {
-		const unsigned int count = eb->buffer_count;
 		unsigned int i;
 
 		for (i = 0; i < count; i++) {
-- 
2.34.1

Re: [PATCH] drm/i915/gem: Prevent overstepping exec array boundary

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] VMA pin leak when returning -EINVAL for parallel contexts after a secure dispatch in `eb_parse`.
--

commit 5f50a42b21cf667f0838c20a62d465078d28fa60
Author: Krzysztof Karas <krzysztof.karas@intel.com>

drm/i915/gem: Prevent overstepping exec array boundary
    
It is possible for eb_relocate_parse_slow() to call kvfree() on
entries outside the original exec array. To amend this problem, ensure
that relocation cleanup uses the original exec object count, excluding
the VMAs appended by the command parser.

Cc: stable@vger.kernel.org # 5.10+
Assisted-by: GitHub-Copilot:gpt-6-astra

> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index 1f303d4eaa4d4..d5aadced7da0c 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -1766,6 +1766,7 @@ static int eb_reinit_userptr(struct i915_execbuffer *eb)
>  
>  static noinline int eb_relocate_parse_slow(struct i915_execbuffer *eb)
>  {
> +	const unsigned int count = eb->buffer_count;
>  	bool have_copy = false;
>  	struct eb_vma *ev;

[ ... ]

> @@ -1868,7 +1869,6 @@ static noinline int eb_relocate_parse_slow(struct i915_execbuffer *eb)
>  
>  out:
>  	if (have_copy) {
> -		const unsigned int count = eb->buffer_count;
>  		unsigned int i;
>  
>  		for (i = 0; i < count; i++) {

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the
command parsing paths, I noticed a potential VMA pin leak in eb_parse()
when handling parallel contexts after a secure dispatch.

In eb_parse(), eb_dispatch_secure() pins a GGTT VMA and returns it as batch.
If intel_context_is_parallel() evaluates to true, the function immediately
returns -EINVAL:

drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:eb_parse() {
	...
secure_batch:
	if (batch) {
		if (intel_context_is_parallel(eb->context))
			return -EINVAL;

		eb->batches[0] = &eb->vma[eb->buffer_count++];
	...
}

Since the early -EINVAL return happens before the pinned batch is added to
the eb->vma array, eb_release_vmas() will not unpin it during the error
cleanup path.

Can this leak the pin count on the GGTT VMA, tying down memory and preventing
the VMA from being unbound? Could this lead to GGTT space exhaustion if
triggered repeatedly?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911061145.3535489-1-krzysztof.karas@intel.com?part=1