linux-kernel - Re: [PATCH] gpu: drm: i915: display: Avoid null values intel_plane_atomic_check_with

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: <87tte1zewf.fsf@intel.com>
Date: Fri, 27 Sep 2024 11:20:32 +0300
From: Jani Nikula <jani.nikula@...ux.intel.com>
To: Alessandro Zanni <alessandro.zanni87@...il.com>, rodrigo.vivi@...el.com,
 joonas.lahtinen@...ux.intel.com, tursulin@...ulin.net, airlied@...il.com,
 simona@...ll.ch
Cc: Alessandro Zanni <alessandro.zanni87@...il.com>,
 intel-gfx@...ts.freedesktop.org, intel-xe@...ts.freedesktop.org,
 dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
 skhan@...uxfoundation.org, anupnewsmail@...il.com, Ville Syrjala
 <ville.syrjala@...ux.intel.com>
Subject: Re: [PATCH] gpu: drm: i915: display: Avoid null values
 intel_plane_atomic_check_with_state

On Fri, 27 Sep 2024, Alessandro Zanni <alessandro.zanni87@...il.com> wrote:
> This fix solves multiple Smatch errors:
>
> drivers/gpu/drm/i915/display/intel_atomic_plane.c:660
> intel_plane_atomic_check_with_state() error:
> we previously assumed 'fb' could be null (see line 648)
>
> drivers/gpu/drm/i915/display/intel_atomic_plane.c:664
> intel_plane_atomic_check_with_state()
> error: we previously assumed 'fb' could be null (see line 659)
>
> drivers/gpu/drm/i915/display/intel_atomic_plane.c:671
> intel_plane_atomic_check_with_state()
> error: we previously assumed 'fb' could be null (see line 663)
>
> We should check first if fb is not null before to access its properties.

new_plane_state->uapi.visible && !fb should not be possible, but it's
probably too hard for smatch to figure out. It's not exactly trivial for
humans to figure out either.

I'm thinking something like below to help both.

Ville, thoughts?


BR,
Jani.


diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index 3505a5b52eb9..d9da47aed55d 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -629,6 +629,9 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_
 	if (ret)
 		return ret;
 
+	if (drm_WARN_ON(display->drm, new_plane_state->uapi.visible && !fb))
+		return -EINVAL;
+
 	if (fb)
 		new_crtc_state->enabled_planes |= BIT(plane->id);
 


>
> Signed-off-by: Alessandro Zanni <alessandro.zanni87@...il.com>
> ---
>  drivers/gpu/drm/i915/display/intel_atomic_plane.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> index e979786aa5cf..1606f79b39e6 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> @@ -656,18 +656,18 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_
>  	    intel_plane_is_scaled(new_plane_state))
>  		new_crtc_state->scaled_planes |= BIT(plane->id);
>  
> -	if (new_plane_state->uapi.visible &&
> +	if (new_plane_state->uapi.visible && fb &&
>  	    intel_format_info_is_yuv_semiplanar(fb->format, fb->modifier))
>  		new_crtc_state->nv12_planes |= BIT(plane->id);
>  
> -	if (new_plane_state->uapi.visible &&
> +	if (new_plane_state->uapi.visible && fb &&
>  	    fb->format->format == DRM_FORMAT_C8)
>  		new_crtc_state->c8_planes |= BIT(plane->id);
>  
>  	if (new_plane_state->uapi.visible || old_plane_state->uapi.visible)
>  		new_crtc_state->update_planes |= BIT(plane->id);
>  
> -	if (new_plane_state->uapi.visible &&
> +	if (new_plane_state->uapi.visible && fb &&
>  	    intel_format_info_is_yuv_semiplanar(fb->format, fb->modifier)) {
>  		new_crtc_state->data_rate_y[plane->id] =
>  			intel_plane_data_rate(new_crtc_state, new_plane_state, 0);

-- 
Jani Nikula, Intel