linux-kernel - Re: [PATCH v3] drm/client: fix null pointer dereference in drm_client_modeset

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 for Android: free password hash cracker in your pocket

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

Message-Id: <20240725013836.1708509-1-make24@iscas.ac.cn>
Date: Thu, 25 Jul 2024 09:38:36 +0800
From: Ma Ke <make24@...as.ac.cn>
To: jani.nikula@...ux.intel.com
Cc: airlied@...il.com,
	daniel@...ll.ch,
	dri-devel@...ts.freedesktop.org,
	linux-kernel@...r.kernel.org,
	maarten.lankhorst@...ux.intel.com,
	make24@...as.ac.cn,
	mripard@...nel.org,
	noralf@...nnes.org,
	sam@...nborg.org,
	stable@...r.kernel.org,
	tzimmermann@...e.de
Subject: Re: [PATCH v3] drm/client: fix null pointer dereference in drm_client_modeset_probe

On Wed, 24 Jul 2024, Jani Nikula <jani.nikula@...ux.intel.com> wrote:
> On Wed, 24 Jul 2024, Ma Ke <make24@...as.ac.cn> wrote:
> > On Wed, 24 Jul 2024, Jani Nikula <jani.nikula@...ux.intel.com> wrote:
> >> On Wed, 24 Jul 2024, Ma Ke <make24@...as.ac.cn> wrote:
> >> > In drm_client_modeset_probe(), the return value of drm_mode_duplicate() is
> >> > assigned to modeset->mode, which will lead to a possible NULL pointer
> >> > dereference on failure of drm_mode_duplicate(). Add a check to avoid npd.
> >> >
> >> > Cc: stable@...r.kernel.org
> >> > Fixes: cf13909aee05 ("drm/fb-helper: Move out modeset config code")
> >> > Signed-off-by: Ma Ke <make24@...as.ac.cn>
> >> > ---
> >> > Changes in v3:
> >> > - modified patch as suggestions, returned error directly when failing to 
> >> > get modeset->mode.
> >> 
> >> This is not what I suggested, and you can't just return here either.
> >> 
> >> BR,
> >> Jani.
> >> 
> >
> > I have carefully read through your comments. Based on your comments on the 
> > patchs I submitted, I am uncertain about the appropriate course of action 
> > following the return value check(whether to continue or to return directly,
> > as both are common approaches in dealing with function drm_mode_duplicate()
> > in Linux kernel, and such handling has received 'acked-by' in similar 
> > vulnerabilities). Could you provide some advice on this matter? Certainly, 
> > adding a return value check is essential, the reasons for which have been 
> > detailed in the vulnerability description. I am looking forward to your 
> > guidance and response. Thank you!
> 
> Everything depends on the context. You can't just go ahead and do the
> same thing everywhere. If you handle errors, even the highly unlikely
> ones such as this one, you better do it properly.
> 
> If you continue here, you'll still leave modeset->mode NULL. And you
> don't propagate the error. Something else is going to hit the issue
> soon.
> 
> If you return directly, you'll leave holding a few locks, and leaking
> memory.
> 
> There's already some error handling in the function, in the same loop
> even. Set ret = -ENOMEM and break.
> 
> (However, you could still argue there's an existing problem in the error
> handling in that all modeset->connectors aren't put and cleaned up.)
> 
> 
> BR,
> Jani.

Indeed, it was my negligence. Thank you very much for your guidance. I will
carefully analyze according to your instructions and resubmit a new patch.

Best regards,

Ma Ke