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

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <571d40f4d3150e61dfb5d2beccdf5c40f3b5be2c.camel@aol.com>
Date: Sat, 03 Jan 2026 19:18:40 +0000
From: Ruben Wauters <rubenru09@....com>
To: Shenghao Yang <me@...nghaoyang.info>, Maarten Lankhorst	
 <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>, 
 Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>,
 Simona Vetter <simona@...ll.ch>
Cc: dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org, 
	stable@...r.kernel.org
Subject: Re: [PATCH] drm/gud: fix NULL fb and crtc dereferences on USB
 disconnect

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

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)