[PATCH 5.10] gpu/i915: fix error return in mmap_offset_attach()

[PATCH 5.10] gpu/i915: fix error return in mmap_offset_attach()

[Alexandr Sapozhnkiov]

From: Alexandr Sapozhnikov <alsp705@gmail.com>

In the drm_vma_node_allow function, kmalloc may 
return NULL, in which case the file element will not be 
added to the mmo->vma_node list. It would be good to 
not ignore this event, but at least log an error message.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Alexandr Sapozhnikov <alsp705@gmail.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_mman.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index a2195e28b625..adaef8f09d59 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -706,8 +706,11 @@ mmap_offset_attach(struct drm_i915_gem_object *obj,
 	mmo = insert_mmo(obj, mmo);
 	GEM_BUG_ON(lookup_mmo(obj, mmap_type) != mmo);
 out:
-	if (file)
-		drm_vma_node_allow_once(&mmo->vma_node, file);
+	if (file) {
+		err = drm_vma_node_allow_once(&mmo->vma_node, file);
+		if (err)
+			goto err;
+	}
 	return mmo;
 
 err:
-- 
2.43.0

✗ LGCI.VerificationFailed: failure for gpu/i915: fix error return in mmap_offset_attach() (rev2)

[Patchwork]

== Series Details ==

Series: gpu/i915: fix error return in mmap_offset_attach() (rev2)
URL   : https://patchwork.freedesktop.org/series/154966/
State : failure

== Summary ==

Address 'alsp705@gmail.com' is not on the allowlist, which prevents CI from being triggered for this patch.
If you want Intel GFX CI to accept this address, please contact the script maintainers at i915-ci-infra@lists.freedesktop.org.
Exception occurred during validation, bailing out!

Re: [PATCH 5.10] gpu/i915: fix error return in mmap_offset_attach()

[Rodrigo Vivi]

On Thu, Oct 02, 2025 at 11:48:26AM +0300, Alexandr Sapozhnkiov wrote:
> From: Alexandr Sapozhnikov <alsp705@gmail.com>

About the subject, this is not just a 5.10 kernel issue.
This code is the current code in our tip.
So this needs to target drm-tip branch, and then Cc stable
and perhaps a Fixes: tag.

> 
> In the drm_vma_node_allow function, kmalloc may 
> return NULL, in which case the file element will not be 
> added to the mmo->vma_node list. It would be good to 
> not ignore this event, but at least log an error message.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Signed-off-by: Alexandr Sapozhnikov <alsp705@gmail.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_mman.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> index a2195e28b625..adaef8f09d59 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> @@ -706,8 +706,11 @@ mmap_offset_attach(struct drm_i915_gem_object *obj,
>  	mmo = insert_mmo(obj, mmo);
>  	GEM_BUG_ON(lookup_mmo(obj, mmap_type) != mmo);
>  out:
> -	if (file)
> -		drm_vma_node_allow_once(&mmo->vma_node, file);
> +	if (file) {
> +		err = drm_vma_node_allow_once(&mmo->vma_node, file);
> +		if (err)

perhaps we also need to drm_vma_offset_remove here?
I mean... honest question, doubt here. Is there any further clean-up needed?

> +			goto err;
> +	}
>  	return mmo;
>  
>  err:
> -- 
> 2.43.0
> 

Re: [PATCH 5.10] gpu/i915: fix error return in mmap_offset_attach()

[Ville Syrjälä]

On Thu, Oct 02, 2025 at 09:55:58AM -0400, Rodrigo Vivi wrote:
> On Thu, Oct 02, 2025 at 11:48:26AM +0300, Alexandr Sapozhnkiov wrote:
> > From: Alexandr Sapozhnikov <alsp705@gmail.com>
> 
> About the subject, this is not just a 5.10 kernel issue.
> This code is the current code in our tip.
> So this needs to target drm-tip branch, and then Cc stable
> and perhaps a Fixes: tag.
> 
> > 
> > In the drm_vma_node_allow function, kmalloc may 
> > return NULL, in which case the file element will not be 
> > added to the mmo->vma_node list. It would be good to 
> > not ignore this event, but at least log an error message.
> > 
> > Found by Linux Verification Center (linuxtesting.org) with SVACE.
> > 
> > Signed-off-by: Alexandr Sapozhnikov <alsp705@gmail.com>
> > ---
> >  drivers/gpu/drm/i915/gem/i915_gem_mman.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> > index a2195e28b625..adaef8f09d59 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> > @@ -706,8 +706,11 @@ mmap_offset_attach(struct drm_i915_gem_object *obj,
> >  	mmo = insert_mmo(obj, mmo);
> >  	GEM_BUG_ON(lookup_mmo(obj, mmap_type) != mmo);
> >  out:
> > -	if (file)
> > -		drm_vma_node_allow_once(&mmo->vma_node, file);
> > +	if (file) {
> > +		err = drm_vma_node_allow_once(&mmo->vma_node, file);
> > +		if (err)
> 
> perhaps we also need to drm_vma_offset_remove here?
> I mean... honest question, doubt here. Is there any further clean-up needed?

Yeah, mmo->vma_node has already been linked to varius places here.
So this will lead to use-after-free in short order.

With the current code if this fails then I think all that ends up
happening is that subsequent mmap() will fail. Maybe that's just
fine?

> 
> > +			goto err;
> > +	}
> >  	return mmo;
> >  
> >  err:
> > -- 
> > 2.43.0
> > 

-- 
Ville Syrjälä
Intel

Re: [PATCH 5.10] gpu/i915: fix error return in mmap_offset_attach()

[Ville Syrjälä]

On Thu, Oct 02, 2025 at 06:20:54PM +0300, Ville Syrjälä wrote:
> On Thu, Oct 02, 2025 at 09:55:58AM -0400, Rodrigo Vivi wrote:
> > On Thu, Oct 02, 2025 at 11:48:26AM +0300, Alexandr Sapozhnkiov wrote:
> > > From: Alexandr Sapozhnikov <alsp705@gmail.com>
> > 
> > About the subject, this is not just a 5.10 kernel issue.
> > This code is the current code in our tip.
> > So this needs to target drm-tip branch, and then Cc stable
> > and perhaps a Fixes: tag.
> > 
> > > 
> > > In the drm_vma_node_allow function, kmalloc may 
> > > return NULL, in which case the file element will not be 
> > > added to the mmo->vma_node list. It would be good to 
> > > not ignore this event, but at least log an error message.
> > > 
> > > Found by Linux Verification Center (linuxtesting.org) with SVACE.
> > > 
> > > Signed-off-by: Alexandr Sapozhnikov <alsp705@gmail.com>
> > > ---
> > >  drivers/gpu/drm/i915/gem/i915_gem_mman.c | 7 +++++--
> > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> > > index a2195e28b625..adaef8f09d59 100644
> > > --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> > > @@ -706,8 +706,11 @@ mmap_offset_attach(struct drm_i915_gem_object *obj,
> > >  	mmo = insert_mmo(obj, mmo);
> > >  	GEM_BUG_ON(lookup_mmo(obj, mmap_type) != mmo);
> > >  out:
> > > -	if (file)
> > > -		drm_vma_node_allow_once(&mmo->vma_node, file);
> > > +	if (file) {
> > > +		err = drm_vma_node_allow_once(&mmo->vma_node, file);
> > > +		if (err)
> > 
> > perhaps we also need to drm_vma_offset_remove here?
> > I mean... honest question, doubt here. Is there any further clean-up needed?
> 
> Yeah, mmo->vma_node has already been linked to varius places here.
> So this will lead to use-after-free in short order.
> 
> With the current code if this fails then I think all that ends up
> happening is that subsequent mmap() will fail. Maybe that's just
> fine?

Hmm, or maybe it would be better to just directly return an error
here without freeing/cleaning up anything. Looks to me like that
should work fine as well, and userspace would get the error
immediately instead of later.

> 
> > 
> > > +			goto err;
> > > +	}
> > >  	return mmo;
> > >  
> > >  err:
> > > -- 
> > > 2.43.0
> > > 
> 
> -- 
> Ville Syrjälä
> Intel

-- 
Ville Syrjälä
Intel

Re: [PATCH 5.10] gpu/i915: fix error return in mmap_offset_attach()

[Andi Shyti]

Hi,

On Thu, Oct 02, 2025 at 11:48:26AM +0300, Alexandr Sapozhnkiov wrote:
> From: Alexandr Sapozhnikov <alsp705@gmail.com>
> 
> In the drm_vma_node_allow function, kmalloc may 
> return NULL, in which case the file element will not be 
> added to the mmo->vma_node list. It would be good to 
> not ignore this event, but at least log an error message.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Signed-off-by: Alexandr Sapozhnikov <alsp705@gmail.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_mman.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> index a2195e28b625..adaef8f09d59 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> @@ -706,8 +706,11 @@ mmap_offset_attach(struct drm_i915_gem_object *obj,
>  	mmo = insert_mmo(obj, mmo);
>  	GEM_BUG_ON(lookup_mmo(obj, mmap_type) != mmo);
>  out:
> -	if (file)
> -		drm_vma_node_allow_once(&mmo->vma_node, file);
> +	if (file) {
> +		err = drm_vma_node_allow_once(&mmo->vma_node, file);
> +		if (err)
> +			goto err;
> +	}

NACK here! You have received several reviews on this patch and
didn't react to them. Please, read carefully the reviews you
received and send a second version of the patch.

Please do use versioning properly in your title and add a
changelog.

Before sending patches, please read the documentation[*].

Andi

[*] https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html

>  	return mmo;
>  
>  err:
> -- 
> 2.43.0
> 

Re: [PATCH 5.10] gpu/i915: fix error return in mmap_offset_attach()

[Petr Vorel]

> Hi,

> On Thu, Oct 02, 2025 at 11:48:26AM +0300, Alexandr Sapozhnkiov wrote:
> > From: Alexandr Sapozhnikov <alsp705@gmail.com>

> > In the drm_vma_node_allow function, kmalloc may 
> > return NULL, in which case the file element will not be 
> > added to the mmo->vma_node list. It would be good to 
> > not ignore this event, but at least log an error message.

> > Found by Linux Verification Center (linuxtesting.org) with SVACE.

> > Signed-off-by: Alexandr Sapozhnikov <alsp705@gmail.com>
> > ---
> >  drivers/gpu/drm/i915/gem/i915_gem_mman.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)

> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> > index a2195e28b625..adaef8f09d59 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> > @@ -706,8 +706,11 @@ mmap_offset_attach(struct drm_i915_gem_object *obj,
> >  	mmo = insert_mmo(obj, mmo);
> >  	GEM_BUG_ON(lookup_mmo(obj, mmap_type) != mmo);
> >  out:
> > -	if (file)
> > -		drm_vma_node_allow_once(&mmo->vma_node, file);
> > +	if (file) {
> > +		err = drm_vma_node_allow_once(&mmo->vma_node, file);
> > +		if (err)
> > +			goto err;
> > +	}

> NACK here! You have received several reviews on this patch and
> didn't react to them. Please, read carefully the reviews you
> received and send a second version of the patch.

> Please do use versioning properly in your title and add a
> changelog.

> Before sending patches, please read the documentation[*].

> Andi

> [*] https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html

Maybe read the latest version.
https://www.kernel.org/doc/html/latest/process/submitting-patches.html

Kind regards,
Petr

> >  	return mmo;

> >  err:
> > -- 
> > 2.43.0

[PATCH 5.10] gpu/i915: fix error return in mmap_offset_attach()

[Alexandr Sapozhnkiov]

From: Alexandr Sapozhnikov <alsp705@gmail.com>

Return value of function 'drm_vma_node_allow', called 
at i915_gem_mman.c:670, is not checked, but it is usually 
checked for this function

Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Alexandr Sapozhnikov <alsp705@gmail.com>

---
 drivers/gpu/drm/i915/gem/i915_gem_mman.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index a2195e28b625..adaef8f09d59 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -706,8 +706,11 @@ mmap_offset_attach(struct drm_i915_gem_object *obj,
 	mmo = insert_mmo(obj, mmo);
 	GEM_BUG_ON(lookup_mmo(obj, mmap_type) != mmo);
 out:
-	if (file)
-		drm_vma_node_allow_once(&mmo->vma_node, file);
+	if (file) {
+		err = drm_vma_node_allow_once(&mmo->vma_node, file);
+		if (err)
+			goto err;
+	}
 	return mmo;
 
 err:
-- 
2.43.0

Re: [PATCH 5.10] gpu/i915: fix error return in mmap_offset_attach()

[Krzysztof Karas]

Hi Alexandr,

> Return value of function 'drm_vma_node_allow', called 
> at i915_gem_mman.c:670, is not checked, but it is usually 
> checked for this function
Grepping for this function in the repo tells me this is currently
the only place we call it. I'd rephrase the commit message to focus
only on missing return code.

After that:
Reviewed-by: Krzysztof Karas <krzysztof.karas@intel.com>
-- 
Best Regards,
Krzysztof

Re: [PATCH 5.10] gpu/i915: fix error return in mmap_offset_attach()

[Александр C]

[-- Attachment #1: Type: text/plain, Size: 1237 bytes --]

Hi Krzysztof Karas,

Ignored (unchecked) return values from functions that can fail.
While a single unchecked return may look harmless, in aggregate they create
a broad
attack surface — a latent, easily-exploitable category of vulnerabilities
that can lead to data corruption,
denial-of-service, or privilege escalation when error conditions are hit in
the field.

Counter example:  /drivers/gpu/drm/drm_prime.c:748:
ret = drm_vma_node_allow(&obj->vma_node, priv);
    if (ret)
        goto out;

Counter example:  /drivers/gpu/drm/drm_gem.c:387:
ret = drm_vma_node_allow(&obj->vma_node, file_priv);
    if (ret)
        goto err_remove;

Best Regards,
Alexandr

вт, 23 сент. 2025 г. в 12:07, Krzysztof Karas <krzysztof.karas@intel.com>:

> Hi Alexandr,
>
> > Return value of function 'drm_vma_node_allow', called
> > at i915_gem_mman.c:670, is not checked, but it is usually
> > checked for this function
> Grepping for this function in the repo tells me this is currently
> the only place we call it. I'd rephrase the commit message to focus
> only on missing return code.
>
> After that:
> Reviewed-by: Krzysztof Karas <krzysztof.karas@intel.com>
> --
> Best Regards,
> Krzysztof
>

[-- Attachment #2: Type: text/html, Size: 3788 bytes --]