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.moriarty@xxxxxxxxxx>
> Reviewed-by: Steven Sistare <steven.sistare@xxxxxxxxxx>
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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
