linux-kernel - Re: [PATCH v2] drm/i915/selftests: Change mock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aF1fPx1U1f1h8_7F@intel.com>
Date: Thu, 26 Jun 2025 10:54:55 -0400
From: Rodrigo Vivi <rodrigo.vivi@...el.com>
To: Dan Carpenter <dan.carpenter@...aro.org>
CC: Jani Nikula <jani.nikula@...ux.intel.com>, Joonas Lahtinen
	<joonas.lahtinen@...ux.intel.com>, Tvrtko Ursulin <tursulin@...ulin.net>,
	David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, "Frederic
 Weisbecker" <frederic@...nel.org>, Chris Wilson <chris@...is-wilson.co.uk>,
	<intel-gfx@...ts.freedesktop.org>, <dri-devel@...ts.freedesktop.org>,
	<linux-kernel@...r.kernel.org>, <kernel-janitors@...r.kernel.org>
Subject: Re: [PATCH v2] drm/i915/selftests: Change mock_request() to return
 error pointers

On Wed, Jun 25, 2025 at 01:39:35PM -0400, Rodrigo Vivi wrote:
> On Wed, Jun 25, 2025 at 10:21:58AM -0500, Dan Carpenter wrote:
> > There was an error pointer vs NULL bug in __igt_breadcrumbs_smoketest().
> > The __mock_request_alloc() function implements the
> > smoketest->request_alloc() function pointer.  It was supposed to return
> > error pointers, but it propogates the NULL return from mock_request()
> > so in the event of a failure, it would lead to a NULL pointer
> > dereference.
> > 
> > To fix this, change the mock_request() function to return error pointers
> > and update all the callers to expect that.
> > 
> > Fixes: 52c0fdb25c7c ("drm/i915: Replace global breadcrumbs with per-context interrupt tracking")
> > Signed-off-by: Dan Carpenter <dan.carpenter@...aro.org>
> > ---
> > V2: In v1 I just updated __mock_request_alloc() to return an error pointer
> >     but in v2, I changed mock_request() to update an error pointer and
> >     updated all the callers.  It's a more extensive change, but hopefully
> >     cleaner.
> 
> Thank you
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@...el.com>

and pushed to drm-intel-next
Thanks again

> 
> 
> > 
> >  drivers/gpu/drm/i915/selftests/i915_request.c | 20 +++++++++----------
> >  drivers/gpu/drm/i915/selftests/mock_request.c |  2 +-
> >  2 files changed, 11 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/selftests/i915_request.c b/drivers/gpu/drm/i915/selftests/i915_request.c
> > index 88870844b5bd..2fb7a9e7efec 100644
> > --- a/drivers/gpu/drm/i915/selftests/i915_request.c
> > +++ b/drivers/gpu/drm/i915/selftests/i915_request.c
> > @@ -73,8 +73,8 @@ static int igt_add_request(void *arg)
> >  	/* Basic preliminary test to create a request and let it loose! */
> >  
> >  	request = mock_request(rcs0(i915)->kernel_context, HZ / 10);
> > -	if (!request)
> > -		return -ENOMEM;
> > +	if (IS_ERR(request))
> > +		return PTR_ERR(request);
> >  
> >  	i915_request_add(request);
> >  
> > @@ -91,8 +91,8 @@ static int igt_wait_request(void *arg)
> >  	/* Submit a request, then wait upon it */
> >  
> >  	request = mock_request(rcs0(i915)->kernel_context, T);
> > -	if (!request)
> > -		return -ENOMEM;
> > +	if (IS_ERR(request))
> > +		return PTR_ERR(request);
> >  
> >  	i915_request_get(request);
> >  
> > @@ -160,8 +160,8 @@ static int igt_fence_wait(void *arg)
> >  	/* Submit a request, treat it as a fence and wait upon it */
> >  
> >  	request = mock_request(rcs0(i915)->kernel_context, T);
> > -	if (!request)
> > -		return -ENOMEM;
> > +	if (IS_ERR(request))
> > +		return PTR_ERR(request);
> >  
> >  	if (dma_fence_wait_timeout(&request->fence, false, T) != -ETIME) {
> >  		pr_err("fence wait success before submit (expected timeout)!\n");
> > @@ -219,8 +219,8 @@ static int igt_request_rewind(void *arg)
> >  	GEM_BUG_ON(IS_ERR(ce));
> >  	request = mock_request(ce, 2 * HZ);
> >  	intel_context_put(ce);
> > -	if (!request) {
> > -		err = -ENOMEM;
> > +	if (IS_ERR(request)) {
> > +		err = PTR_ERR(request);
> >  		goto err_context_0;
> >  	}
> >  
> > @@ -237,8 +237,8 @@ static int igt_request_rewind(void *arg)
> >  	GEM_BUG_ON(IS_ERR(ce));
> >  	vip = mock_request(ce, 0);
> >  	intel_context_put(ce);
> > -	if (!vip) {
> > -		err = -ENOMEM;
> > +	if (IS_ERR(vip)) {
> > +		err = PTR_ERR(vip);
> >  		goto err_context_1;
> >  	}
> >  
> > diff --git a/drivers/gpu/drm/i915/selftests/mock_request.c b/drivers/gpu/drm/i915/selftests/mock_request.c
> > index 09f747228dff..1b0cf073e964 100644
> > --- a/drivers/gpu/drm/i915/selftests/mock_request.c
> > +++ b/drivers/gpu/drm/i915/selftests/mock_request.c
> > @@ -35,7 +35,7 @@ mock_request(struct intel_context *ce, unsigned long delay)
> >  	/* NB the i915->requests slab cache is enlarged to fit mock_request */
> >  	request = intel_context_create_request(ce);
> >  	if (IS_ERR(request))
> > -		return NULL;
> > +		return request;
> >  
> >  	request->mock.delay = delay;
> >  	return request;
> > -- 
> > 2.47.2
> >