On Mon, Feb 12, 2018 at 02:51:44PM -0500, Joe Moriarty wrote:
> The Parfait (version 2.1.0) static code analysis tool found the
> following NULL pointer derefernce problem.
> 
> - drivers/gpu/drm/drm_vblank.c
> Null pointer checks were added to return values from calls to
> drm_crtc_from_index().  There is a possibility, however minute, that
> crtc->index may not be found when trying to find the struct crtc
> from it's assigned index given in drm_crtc_init_with_planes().
> 3 return checks for NULL where added.
> 
> Signed-off-by: Joe Moriarty <joe.moria...@oracle.com>
> Reviewed-by: Steven Sistare <steven.sist...@oracle.com>

These are all drivers bugs, we'd need at least a WARN_ON when the crtc
doesn't exist. Otherwise this would just silently paper over a fairly
serious kernel bug (which doesn't improve things really).

Something like

        if (WARN_ON(!crtc))
                return NULL;

is what I'd go with.

-Daniel

> ---
>  drivers/gpu/drm/drm_vblank.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 32d9bcf5be7f..a3a1bce87468 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -120,7 +120,7 @@ static u32 __get_vblank_counter(struct drm_device *dev, 
> unsigned int pipe)
>       if (drm_core_check_feature(dev, DRIVER_MODESET)) {
>               struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe);
>  
> -             if (crtc->funcs->get_vblank_counter)
> +             if (crtc && crtc->funcs->get_vblank_counter)
>                       return crtc->funcs->get_vblank_counter(crtc);
>       }
>  
> @@ -318,7 +318,7 @@ static void __disable_vblank(struct drm_device *dev, 
> unsigned int pipe)
>       if (drm_core_check_feature(dev, DRIVER_MODESET)) {
>               struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe);
>  
> -             if (crtc->funcs->disable_vblank) {
> +             if (crtc && crtc->funcs->disable_vblank) {
>                       crtc->funcs->disable_vblank(crtc);
>                       return;
>               }
> @@ -918,7 +918,7 @@ static int __enable_vblank(struct drm_device *dev, 
> unsigned int pipe)
>       if (drm_core_check_feature(dev, DRIVER_MODESET)) {
>               struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe);
>  
> -             if (crtc->funcs->enable_vblank)
> +             if (crtc && crtc->funcs->enable_vblank)
>                       return crtc->funcs->enable_vblank(crtc);
>       }
>  
> -- 
> 2.15.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to