[tiL4.19-AD PATCH 1/2] drm/tilcdc: Check drm_fb_cma_get_gem_obj() return value

[Laurent Pinchart]

On Tue, Mar 05, 2019 at 09:42:50PM +0200, Jyri Sarha wrote:
> On 05/03/2019 17:43, Laurent Pinchart wrote:
> > Hi Jyri,
> > 
> > Thank you for the patch.
> > 
> > On Thu, Feb 28, 2019 at 01:18:50PM +0200, Jyri Sarha wrote:
> >> drm_fb_cma_get_gem_obj() may return NULL. The return value needs to be
> >> checked before dereferencing the returned pointer.
> >>
> >> Signed-off-by: Jyri Sarha <jsarha at ti.com>
> >> ---
> >>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> >> index 1067e702c22c..a8ae064f52bb 100644
> >> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> >> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> >> @@ -75,6 +75,8 @@ static void set_scanout(struct drm_crtc *crtc, struct drm_framebuffer *fb)
> >>  	u64 dma_base_and_ceiling;
> >>  
> >>  	gem = drm_fb_cma_get_gem_obj(fb, 0);
> >> +	if (WARN_ON(!gem))
> >> +		return;
> > 
> > But this should not happen, right ? Don't we have the required checks in
> > place to ensure there will always be a valid GEM object available here ?
> > 
> 
> The patch is trying to make clockwork - a static analysis tool - happy.
> Should have mentioned that in the patch. But the fact that static
> clockwork complains about this suggests that either there is no such
> check elsewhere, or clockwork does not understand it.

I'd go for the latter. drm_fb_cma_get_gem_obj() will return NULL if the
plane index is >= 4 (which is clearly not the case here), and will
otherwise return the GEM object pointer from an internal array. That
array is populated when the framebuffer is allocated created, by
drm_gem_fb_create() called from tilcdc_fb_create() in this case. The
check here is not needed, but clockwork can't easily know about that.

> >>  	start = gem->paddr + fb->offsets[0] +
> >>  		crtc->y * fb->pitches[0] +

-- 
Regards,

Laurent Pinchart