[Intel-gfx] [bug report] drm/i915: Fix enabled_planes bitmask

[Intel-gfx] [bug report] drm/i915: Fix enabled_planes bitmask

[Dan Carpenter]

Hello Ville Syrjälä,

This is a semi-automatic email about new static checker warnings.

The patch 97bc7ffa1b1e: "drm/i915: Fix enabled_planes bitmask" from
Mar 5, 2021, leads to the following Smatch complaint:

    drivers/gpu/drm/i915/display/intel_atomic_plane.c:332 intel_plane_atomic_check_with_state()
    error: we previously assumed 'fb' could be null (see line 324)

drivers/gpu/drm/i915/display/intel_atomic_plane.c
   323	
   324		if (fb)
                    ^^
The patch adds a check for NULL

   325			new_crtc_state->enabled_planes |= BIT(plane->id);
   326	
   327		/* FIXME pre-g4x don't work like this */
   328		if (new_plane_state->uapi.visible)
   329			new_crtc_state->active_planes |= BIT(plane->id);
   330	
   331		if (new_plane_state->uapi.visible &&
   332		    intel_format_info_is_yuv_semiplanar(fb->format, fb->modifier))
                                                        ^^^^^^^^^^^^^^^^^^^^^^^^
Unchecked deref

   333			new_crtc_state->nv12_planes |= BIT(plane->id);
   334	
   335          if (new_plane_state->uapi.visible &&
   336              fb->format->format == DRM_FORMAT_C8)
                    ^^^^^^^^^^
Same.  These are potentially false positives.  I don't know if
"uapi.visible" can be true when ctrc is non-NULL and fb is NULL for
example?  (The point is I read the commit message, but I know pants all
about DRM.  :P)

   337                  new_crtc_state->c8_planes |= BIT(plane->id);
   338  
   339          if (new_plane_state->uapi.visible || old_plane_state->uapi.visible)
   340                  new_crtc_state->update_planes |= BIT(plane->id);
   341  
   342          new_crtc_state->data_rate[plane->id] =
   343                  intel_plane_data_rate(new_crtc_state, new_plane_state);
   344  
   345          return intel_plane_atomic_calc_changes(old_crtc_state, new_crtc_state,
   346                                                 old_plane_state, new_plane_state);
   347  }

regards,
dan carpenter
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [bug report] drm/i915: Fix enabled_planes bitmask

[Ville Syrjälä]

On Wed, Mar 24, 2021 at 10:22:54AM +0300, Dan Carpenter wrote:
> Hello Ville Syrjälä,
> 
> This is a semi-automatic email about new static checker warnings.
> 
> The patch 97bc7ffa1b1e: "drm/i915: Fix enabled_planes bitmask" from
> Mar 5, 2021, leads to the following Smatch complaint:
> 
>     drivers/gpu/drm/i915/display/intel_atomic_plane.c:332 intel_plane_atomic_check_with_state()
>     error: we previously assumed 'fb' could be null (see line 324)
> 
> drivers/gpu/drm/i915/display/intel_atomic_plane.c
>    323	
>    324		if (fb)
>                     ^^
> The patch adds a check for NULL
> 
>    325			new_crtc_state->enabled_planes |= BIT(plane->id);
>    326	
>    327		/* FIXME pre-g4x don't work like this */
>    328		if (new_plane_state->uapi.visible)
>    329			new_crtc_state->active_planes |= BIT(plane->id);
>    330	
>    331		if (new_plane_state->uapi.visible &&
>    332		    intel_format_info_is_yuv_semiplanar(fb->format, fb->modifier))
>                                                         ^^^^^^^^^^^^^^^^^^^^^^^^
> Unchecked deref
> 
>    333			new_crtc_state->nv12_planes |= BIT(plane->id);
>    334	
>    335          if (new_plane_state->uapi.visible &&
>    336              fb->format->format == DRM_FORMAT_C8)
>                     ^^^^^^^^^^
> Same.  These are potentially false positives.  I don't know if
> "uapi.visible" can be true when ctrc is non-NULL and fb is NULL for
> example?  (The point is I read the commit message, but I know pants all
> about DRM.  :P)

If there's no fb then uapi.visible=false. See
intel_atomic_plane_check_clipping().

> 
>    337                  new_crtc_state->c8_planes |= BIT(plane->id);
>    338  
>    339          if (new_plane_state->uapi.visible || old_plane_state->uapi.visible)
>    340                  new_crtc_state->update_planes |= BIT(plane->id);
>    341  
>    342          new_crtc_state->data_rate[plane->id] =
>    343                  intel_plane_data_rate(new_crtc_state, new_plane_state);
>    344  
>    345          return intel_plane_atomic_calc_changes(old_crtc_state, new_crtc_state,
>    346                                                 old_plane_state, new_plane_state);
>    347  }
> 
> regards,
> dan carpenter

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx