[PATCH] drm/gud: fix NULL fb and crtc dereferences on USB disconnect

Shenghao Yang posted 1 patch 1 month, 1 week ago
drivers/gpu/drm/gud/gud_pipe.c | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)
[PATCH] drm/gud: fix NULL fb and crtc dereferences on USB disconnect
Posted by Shenghao Yang 1 month, 1 week ago
On disconnect drm_atomic_helper_disable_all() is called which
sets both the fb and crtc for a plane to NULL before invoking a commit.

This causes a kernel oops on every display disconnect.

Add guards for those dereferences.

Cc: <stable@vger.kernel.org> # 6.18.x
Signed-off-by: Shenghao Yang <me@shenghaoyang.info>
---
 drivers/gpu/drm/gud/gud_pipe.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
index 76d77a736d84..4b77be94348d 100644
--- a/drivers/gpu/drm/gud/gud_pipe.c
+++ b/drivers/gpu/drm/gud/gud_pipe.c
@@ -457,27 +457,20 @@ int gud_plane_atomic_check(struct drm_plane *plane,
 	struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
 	struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane);
 	struct drm_crtc *crtc = new_plane_state->crtc;
-	struct drm_crtc_state *crtc_state;
+	struct drm_crtc_state *crtc_state = NULL;
 	const struct drm_display_mode *mode;
 	struct drm_framebuffer *old_fb = old_plane_state->fb;
 	struct drm_connector_state *connector_state = NULL;
 	struct drm_framebuffer *fb = new_plane_state->fb;
-	const struct drm_format_info *format = fb->format;
+	const struct drm_format_info *format;
 	struct drm_connector *connector;
 	unsigned int i, num_properties;
 	struct gud_state_req *req;
 	int idx, ret;
 	size_t len;
 
-	if (drm_WARN_ON_ONCE(plane->dev, !fb))
-		return -EINVAL;
-
-	if (drm_WARN_ON_ONCE(plane->dev, !crtc))
-		return -EINVAL;
-
-	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
-
-	mode = &crtc_state->mode;
+	if (crtc)
+		crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
 
 	ret = drm_atomic_helper_check_plane_state(new_plane_state, crtc_state,
 						  DRM_PLANE_NO_SCALING,
@@ -492,6 +485,9 @@ int gud_plane_atomic_check(struct drm_plane *plane,
 	if (old_plane_state->rotation != new_plane_state->rotation)
 		crtc_state->mode_changed = true;
 
+	mode = &crtc_state->mode;
+	format = fb->format;
+
 	if (old_fb && old_fb->format != format)
 		crtc_state->mode_changed = true;
 
@@ -598,7 +594,7 @@ void gud_plane_atomic_update(struct drm_plane *plane,
 	struct drm_atomic_helper_damage_iter iter;
 	int ret, idx;
 
-	if (crtc->state->mode_changed || !crtc->state->enable) {
+	if (!crtc || crtc->state->mode_changed || !crtc->state->enable) {
 		cancel_work_sync(&gdrm->work);
 		mutex_lock(&gdrm->damage_lock);
 		if (gdrm->fb) {
-- 
2.52.0
Re: [PATCH] drm/gud: fix NULL fb and crtc dereferences on USB disconnect
Posted by Ruben Wauters 1 month ago
Hi

On Wed, 2025-12-31 at 13:50 +0800, Shenghao Yang wrote:
> On disconnect drm_atomic_helper_disable_all() is called which
> sets both the fb and crtc for a plane to NULL before invoking a commit.
> 
> This causes a kernel oops on every display disconnect.
> 
> Add guards for those dereferences.
> 
> Cc: <stable@vger.kernel.org> # 6.18.x
> Signed-off-by: Shenghao Yang <me@shenghaoyang.info>
> ---
>  drivers/gpu/drm/gud/gud_pipe.c | 20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
> index 76d77a736d84..4b77be94348d 100644
> --- a/drivers/gpu/drm/gud/gud_pipe.c
> +++ b/drivers/gpu/drm/gud/gud_pipe.c
> @@ -457,27 +457,20 @@ int gud_plane_atomic_check(struct drm_plane *plane,
>  	struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
>  	struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane);
>  	struct drm_crtc *crtc = new_plane_state->crtc;
> -	struct drm_crtc_state *crtc_state;
> +	struct drm_crtc_state *crtc_state = NULL;
>  	const struct drm_display_mode *mode;
>  	struct drm_framebuffer *old_fb = old_plane_state->fb;
>  	struct drm_connector_state *connector_state = NULL;
>  	struct drm_framebuffer *fb = new_plane_state->fb;
> -	const struct drm_format_info *format = fb->format;
> +	const struct drm_format_info *format;
>  	struct drm_connector *connector;
>  	unsigned int i, num_properties;
>  	struct gud_state_req *req;
>  	int idx, ret;
>  	size_t len;
>  
> -	if (drm_WARN_ON_ONCE(plane->dev, !fb))
> -		return -EINVAL;
> -
> -	if (drm_WARN_ON_ONCE(plane->dev, !crtc))
> -		return -EINVAL;

With the elimination of these two WARN_ON_ONCEs, it's possible that
crtc_state may not be assigned below, and therefore may be read/passed
to functions when it is NULL (e.g. line 488). Either protection for a
null crtc_state should be added to the rest of the function, or the
function shouldn't continue if crtc is NULL.

Ruben
> -	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
> -
> -	mode = &crtc_state->mode;
> +	if (crtc)
> +		crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
>  
>  	ret = drm_atomic_helper_check_plane_state(new_plane_state, crtc_state,
>  						  DRM_PLANE_NO_SCALING,
> @@ -492,6 +485,9 @@ int gud_plane_atomic_check(struct drm_plane *plane,
>  	if (old_plane_state->rotation != new_plane_state->rotation)
>  		crtc_state->mode_changed = true;
>  
> +	mode = &crtc_state->mode;
> +	format = fb->format;
> +
>  	if (old_fb && old_fb->format != format)
>  		crtc_state->mode_changed = true;
>  
> @@ -598,7 +594,7 @@ void gud_plane_atomic_update(struct drm_plane *plane,
>  	struct drm_atomic_helper_damage_iter iter;
>  	int ret, idx;
>  
> -	if (crtc->state->mode_changed || !crtc->state->enable) {
> +	if (!crtc || crtc->state->mode_changed || !crtc->state->enable) {
>  		cancel_work_sync(&gdrm->work);
>  		mutex_lock(&gdrm->damage_lock);
>  		if (gdrm->fb) {
Re: [PATCH] drm/gud: fix NULL fb and crtc dereferences on USB disconnect
Posted by Shenghao Yang 1 month ago
Hi Ruben,

On 4/1/26 01:23, Ruben Wauters wrote:

> With the elimination of these two WARN_ON_ONCEs, it's possible that
> crtc_state may not be assigned below, and therefore may be read/passed
> to functions when it is NULL (e.g. line 488). Either protection for a
> null crtc_state should be added to the rest of the function, or the
> function shouldn't continue if crtc is NULL.
> 
> Ruben
>> -	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
>> -
>> -	mode = &crtc_state->mode;
>> +	if (crtc)
>> +		crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
>>  
>>  	ret = drm_atomic_helper_check_plane_state(new_plane_state, crtc_state,
>>  						  DRM_PLANE_NO_SCALING,
>> @@ -492,6 +485,9 @@ int gud_plane_atomic_check(struct drm_plane *plane,
>>  	if (old_plane_state->rotation != new_plane_state->rotation)
>>  		crtc_state->mode_changed = true;
>>  
>> +	mode = &crtc_state->mode;
>> +	format = fb->format;

Yup - in this case I'm relying on drm_atomic_helper_check_plane_state()
bailing out early after seeing that fb is NULL (since a NULL crtc should
imply no fb) and setting plane_state->visible to false.

That would cause an early return in gud_plane_atomic_check() without
dereferencing crtc_state.

Would a more explicit check be preferred?

Thanks,

Shenghao
Re: [PATCH] drm/gud: fix NULL fb and crtc dereferences on USB disconnect
Posted by Ruben Wauters 1 month ago
Hi

On Sun, 2026-01-04 at 01:47 +0800, Shenghao Yang wrote:
> Hi Ruben,
> 
> On 4/1/26 01:23, Ruben Wauters wrote:
> 
> > With the elimination of these two WARN_ON_ONCEs, it's possible that
> > crtc_state may not be assigned below, and therefore may be read/passed
> > to functions when it is NULL (e.g. line 488). Either protection for a
> > null crtc_state should be added to the rest of the function, or the
> > function shouldn't continue if crtc is NULL.
> > 
> > Ruben
> > > -	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
> > > -
> > > -	mode = &crtc_state->mode;
> > > +	if (crtc)
> > > +		crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
> > >  
> > >  	ret = drm_atomic_helper_check_plane_state(new_plane_state, crtc_state,
> > >  						  DRM_PLANE_NO_SCALING,
> > > @@ -492,6 +485,9 @@ int gud_plane_atomic_check(struct drm_plane *plane,
> > >  	if (old_plane_state->rotation != new_plane_state->rotation)
> > >  		crtc_state->mode_changed = true;
> > >  
> > > +	mode = &crtc_state->mode;
> > > +	format = fb->format;
> 
> Yup - in this case I'm relying on drm_atomic_helper_check_plane_state()
> bailing out early after seeing that fb is NULL (since a NULL crtc should
> imply no fb) and setting plane_state->visible to false.
> 
> That would cause an early return in gud_plane_atomic_check() without
> dereferencing crtc_state.

This does work, however this ends up returning 0, which implies that
the atomic check succeded. In my opinion in this case, -EINVAL should
be returned, as both the crtc and fb don't exist, therefore the check
should not succeed. I would personally prefer a more explicit check
that does return -EINVAL instead of 0 from
drm_atomic_helper_check_planes()

As a side note, I'm not sure if there's a reasoning as to why
drm_atomic_helper_check_planes() returns 0 if fb is NULL instead of 
-EINVAL, I'm assuming it's not designed to come across this specific
case. Either way it's not too much of an issue but maybe one of the drm
maintainers can clarify why it's this way.

Ruben
> 
> Would a more explicit check be preferred?
> 
> Thanks,
> 
> Shenghao
Re: [PATCH] drm/gud: fix NULL fb and crtc dereferences on USB disconnect
Posted by Thomas Zimmermann 1 month ago
Hi Ruben

Am 03.01.26 um 20:18 schrieb Ruben Wauters:
> Hi
>
> On Sun, 2026-01-04 at 01:47 +0800, Shenghao Yang wrote:
>> Hi Ruben,
>>
>> On 4/1/26 01:23, Ruben Wauters wrote:
>>
>>> With the elimination of these two WARN_ON_ONCEs, it's possible that
>>> crtc_state may not be assigned below, and therefore may be read/passed
>>> to functions when it is NULL (e.g. line 488). Either protection for a
>>> null crtc_state should be added to the rest of the function, or the
>>> function shouldn't continue if crtc is NULL.
>>>
>>> Ruben
>>>> -	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
>>>> -
>>>> -	mode = &crtc_state->mode;
>>>> +	if (crtc)
>>>> +		crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
>>>>   
>>>>   	ret = drm_atomic_helper_check_plane_state(new_plane_state, crtc_state,
>>>>   						  DRM_PLANE_NO_SCALING,
>>>> @@ -492,6 +485,9 @@ int gud_plane_atomic_check(struct drm_plane *plane,
>>>>   	if (old_plane_state->rotation != new_plane_state->rotation)
>>>>   		crtc_state->mode_changed = true;
>>>>   
>>>> +	mode = &crtc_state->mode;
>>>> +	format = fb->format;
>> Yup - in this case I'm relying on drm_atomic_helper_check_plane_state()
>> bailing out early after seeing that fb is NULL (since a NULL crtc should
>> imply no fb) and setting plane_state->visible to false.

This is correct behavior.

>>
>> That would cause an early return in gud_plane_atomic_check() without
>> dereferencing crtc_state.
> This does work, however this ends up returning 0, which implies that
> the atomic check succeded. In my opinion in this case, -EINVAL should
> be returned, as both the crtc and fb don't exist, therefore the check
> should not succeed. I would personally prefer a more explicit check
> that does return -EINVAL instead of 0 from
> drm_atomic_helper_check_planes()

If the plane has been disbabled, fb and crtc are NULL. So this is 
correct. drm_atomic_helper_check_plane_state() is the place to do this 
testing before doing much else in the atomic_check handler. If 
drm_atomic_helper_check_plane_state() gives an error, the error should 
be returns. But if it returns 0 and sets ->visible to false, the 
atomic_check should return 0.  That means that the plane can 
successfully be disabled.

>
> As a side note, I'm not sure if there's a reasoning as to why
> drm_atomic_helper_check_planes() returns 0 if fb is NULL instead of
> -EINVAL, I'm assuming it's not designed to come across this specific
> case. Either way it's not too much of an issue but maybe one of the drm
> maintainers can clarify why it's this way.

Disabling a plane is not an error, but a common operation.

I think the patch is fine and IIRC we have similar logic in other drivers.

Best regards
Thomas

>
> Ruben
>> Would a more explicit check be preferred?
>>
>> Thanks,
>>
>> Shenghao

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)


Re: [PATCH] drm/gud: fix NULL fb and crtc dereferences on USB disconnect
Posted by Ruben Wauters 1 month ago
Hi,

On Wed, 2026-01-07 at 08:46 +0100, Thomas Zimmermann wrote:
> Hi Ruben
> 
> Am 03.01.26 um 20:18 schrieb Ruben Wauters:
> > Hi
> > 
> > On Sun, 2026-01-04 at 01:47 +0800, Shenghao Yang wrote:
> > > Hi Ruben,
> > > 
> > > On 4/1/26 01:23, Ruben Wauters wrote:
> > > 
> > > > With the elimination of these two WARN_ON_ONCEs, it's possible that
> > > > crtc_state may not be assigned below, and therefore may be read/passed
> > > > to functions when it is NULL (e.g. line 488). Either protection for a
> > > > null crtc_state should be added to the rest of the function, or the
> > > > function shouldn't continue if crtc is NULL.
> > > > 
> > > > Ruben
> > > > > -	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
> > > > > -
> > > > > -	mode = &crtc_state->mode;
> > > > > +	if (crtc)
> > > > > +		crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
> > > > >   
> > > > >   	ret = drm_atomic_helper_check_plane_state(new_plane_state, crtc_state,
> > > > >   						  DRM_PLANE_NO_SCALING,
> > > > > @@ -492,6 +485,9 @@ int gud_plane_atomic_check(struct drm_plane *plane,
> > > > >   	if (old_plane_state->rotation != new_plane_state->rotation)
> > > > >   		crtc_state->mode_changed = true;
> > > > >   
> > > > > +	mode = &crtc_state->mode;
> > > > > +	format = fb->format;
> > > Yup - in this case I'm relying on drm_atomic_helper_check_plane_state()
> > > bailing out early after seeing that fb is NULL (since a NULL crtc should
> > > imply no fb) and setting plane_state->visible to false.
> 
> This is correct behavior.
> 
> > > 
> > > That would cause an early return in gud_plane_atomic_check() without
> > > dereferencing crtc_state.
> > This does work, however this ends up returning 0, which implies that
> > the atomic check succeded. In my opinion in this case, -EINVAL should
> > be returned, as both the crtc and fb don't exist, therefore the check
> > should not succeed. I would personally prefer a more explicit check
> > that does return -EINVAL instead of 0 from
> > drm_atomic_helper_check_planes()
> 
> If the plane has been disbabled, fb and crtc are NULL. So this is 
> correct. drm_atomic_helper_check_plane_state() is the place to do this 
> testing before doing much else in the atomic_check handler. If 
> drm_atomic_helper_check_plane_state() gives an error, the error should 
> be returns. But if it returns 0 and sets ->visible to false, the 
> atomic_check should return 0.  That means that the plane can 
> successfully be disabled.
> 
> > 
> > As a side note, I'm not sure if there's a reasoning as to why
> > drm_atomic_helper_check_planes() returns 0 if fb is NULL instead of
> > -EINVAL, I'm assuming it's not designed to come across this specific
> > case. Either way it's not too much of an issue but maybe one of the drm
> > maintainers can clarify why it's this way.
> 
> Disabling a plane is not an error, but a common operation.

I think I may have misunderstood the drm docs on this, if having crtc
and fb be null and returning 0 then is ok, I am happy for this patch to
be applied. I have tested it and it indeed works, and removes the oops
present in the driver before this.
> 
> I think the patch is fine and IIRC we have similar logic in other drivers.

Reviewed-by: Ruben Wauters <rubenru09@aol.com>

I believe Shenghao mentioned another oops that is present? if so it may
be best to submit that in a separate patch rather than a v2 of this
one.

Ruben
> 
> Best regards
> Thomas
> 
> > 
> > Ruben
> > > Would a more explicit check be preferred?
> > > 
> > > Thanks,
> > > 
> > > Shenghao
Re: [PATCH] drm/gud: fix NULL fb and crtc dereferences on USB disconnect
Posted by Thomas Zimmermann 1 month ago
Hi

Am 07.01.26 um 16:02 schrieb Ruben Wauters:
> Hi,
>
> On Wed, 2026-01-07 at 08:46 +0100, Thomas Zimmermann wrote:
>> Hi Ruben
>>
>> Am 03.01.26 um 20:18 schrieb Ruben Wauters:
>>> Hi
>>>
>>> On Sun, 2026-01-04 at 01:47 +0800, Shenghao Yang wrote:
>>>> Hi Ruben,
>>>>
>>>> On 4/1/26 01:23, Ruben Wauters wrote:
>>>>
>>>>> With the elimination of these two WARN_ON_ONCEs, it's possible that
>>>>> crtc_state may not be assigned below, and therefore may be read/passed
>>>>> to functions when it is NULL (e.g. line 488). Either protection for a
>>>>> null crtc_state should be added to the rest of the function, or the
>>>>> function shouldn't continue if crtc is NULL.
>>>>>
>>>>> Ruben
>>>>>> -	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
>>>>>> -
>>>>>> -	mode = &crtc_state->mode;
>>>>>> +	if (crtc)
>>>>>> +		crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
>>>>>>    
>>>>>>    	ret = drm_atomic_helper_check_plane_state(new_plane_state, crtc_state,
>>>>>>    						  DRM_PLANE_NO_SCALING,
>>>>>> @@ -492,6 +485,9 @@ int gud_plane_atomic_check(struct drm_plane *plane,
>>>>>>    	if (old_plane_state->rotation != new_plane_state->rotation)
>>>>>>    		crtc_state->mode_changed = true;
>>>>>>    
>>>>>> +	mode = &crtc_state->mode;
>>>>>> +	format = fb->format;
>>>> Yup - in this case I'm relying on drm_atomic_helper_check_plane_state()
>>>> bailing out early after seeing that fb is NULL (since a NULL crtc should
>>>> imply no fb) and setting plane_state->visible to false.
>> This is correct behavior.
>>
>>>> That would cause an early return in gud_plane_atomic_check() without
>>>> dereferencing crtc_state.
>>> This does work, however this ends up returning 0, which implies that
>>> the atomic check succeded. In my opinion in this case, -EINVAL should
>>> be returned, as both the crtc and fb don't exist, therefore the check
>>> should not succeed. I would personally prefer a more explicit check
>>> that does return -EINVAL instead of 0 from
>>> drm_atomic_helper_check_planes()
>> If the plane has been disbabled, fb and crtc are NULL. So this is
>> correct. drm_atomic_helper_check_plane_state() is the place to do this
>> testing before doing much else in the atomic_check handler. If
>> drm_atomic_helper_check_plane_state() gives an error, the error should
>> be returns. But if it returns 0 and sets ->visible to false, the
>> atomic_check should return 0.  That means that the plane can
>> successfully be disabled.
>>
>>> As a side note, I'm not sure if there's a reasoning as to why
>>> drm_atomic_helper_check_planes() returns 0 if fb is NULL instead of
>>> -EINVAL, I'm assuming it's not designed to come across this specific
>>> case. Either way it's not too much of an issue but maybe one of the drm
>>> maintainers can clarify why it's this way.
>> Disabling a plane is not an error, but a common operation.
> I think I may have misunderstood the drm docs on this, if having crtc
> and fb be null and returning 0 then is ok, I am happy for this patch to
> be applied. I have tested it and it indeed works, and removes the oops
> present in the driver before this.

No worries, DRM semantics can be murky. This is one of the cases that is 
impossible to know unless you came across a patch like this one.

Best regards
Thomas

>> I think the patch is fine and IIRC we have similar logic in other drivers.
> Reviewed-by: Ruben Wauters <rubenru09@aol.com>
>
> I believe Shenghao mentioned another oops that is present? if so it may
> be best to submit that in a separate patch rather than a v2 of this
> one.
>
> Ruben
>> Best regards
>> Thomas
>>
>>> Ruben
>>>> Would a more explicit check be preferred?
>>>>
>>>> Thanks,
>>>>
>>>> Shenghao

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)


Re: [PATCH] drm/gud: fix NULL fb and crtc dereferences on USB disconnect
Posted by Shenghao Yang 4 weeks, 1 day ago
Hi Ruben, Thomas,

On 7/1/26 23:56, Thomas Zimmermann wrote:

> 
> No worries, DRM semantics can be murky. This is one of the cases that is impossible to know unless you came across a patch like this one.
> 
> Best regards
> Thomas
> 
>>> I think the patch is fine and IIRC we have similar logic in other drivers.
>> Reviewed-by: Ruben Wauters <rubenru09@aol.com>
>>
>> I believe Shenghao mentioned another oops that is present? if so it may
>> be best to submit that in a separate patch rather than a v2 of this
>> one.
>>
>> Ruben


Thanks both! I'll split the patch for the second oops.

Shenghao
Re: [PATCH] drm/gud: fix NULL fb and crtc dereferences on USB disconnect
Posted by Ruben Wauters 3 weeks, 3 days ago
On Thu, 2026-01-08 at 21:39 +0800, Shenghao Yang wrote
Hi
> Hi Ruben, Thomas,
> 
> On 7/1/26 23:56, Thomas Zimmermann wrote:
> 
> > 
> > No worries, DRM semantics can be murky. This is one of the cases that is impossible to know unless you came across a patch like this one.
> > 
> > Best regards
> > Thomas
> > 
> > > > I think the patch is fine and IIRC we have similar logic in other drivers.
> > > Reviewed-by: Ruben Wauters <rubenru09@aol.com>

Patch applied to drm-misc-fixes, thanks
> > > 
> > > I believe Shenghao mentioned another oops that is present? if so it may
> > > be best to submit that in a separate patch rather than a v2 of this
> > > one.
> > > 
> > > Ruben
> 
> 
> Thanks both! I'll split the patch for the second oops.
> 
> Shenghao
Re: [PATCH] drm/gud: fix NULL fb and crtc dereferences on USB disconnect
Posted by Shenghao Yang 1 month ago
Hi Ruben,

On 4/1/26 03:18, Ruben Wauters wrote:
> Hi
> 
> This does work, however this ends up returning 0, which implies that
> the atomic check succeded. In my opinion in this case, -EINVAL should
> be returned, as both the crtc and fb don't exist, therefore the check
> should not succeed. I would personally prefer a more explicit check
> that does return -EINVAL instead of 0 from
> drm_atomic_helper_check_planes()

> As a side note, I'm not sure if there's a reasoning as to why
> drm_atomic_helper_check_planes() returns 0 if fb is NULL instead of 
> -EINVAL, I'm assuming it's not designed to come across this specific
> case. Either way it's not too much of an issue but maybe one of the drm
> maintainers can clarify why it's this way.

Maybe this is a result of the atomic conversions? It's possible that
now we get passed NULLs on hotplug and display disables. (I didn't know
enough about DRM to be sure and didn't reference that commit in the
previous email).

I think a return of 0 should be it - both exynos_plane_atomic_check()[1] and
virtio_gpu_plane_atomic_check()[2] return 0 on either a NULL fb or crtc -
I've tried returning -EINVAL and KDE can no longer disable the display
because the rejection is being propagated back to userspace.

I'll respin this patch to return 0 after an explicit check and include
another NULL dereference fix in the plane update path.

Thanks,

Shenghao

[1] https://elixir.bootlin.com/linux/v6.18.2/source/drivers/gpu/drm/exynos/exynos_drm_plane.c#L231
[2] https://elixir.bootlin.com/linux/v6.18.2/C/ident/virtio_gpu_plane_atomic_check