dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Jyri Sarha <jsarha@ti.com>
Cc: peter.ujfalusi@ti.com, tomi.valkeinen@ti.com,
	dri-devel@lists.freedesktop.org
Subject: Re: [tiL4.19-AD PATCH 1/2] drm/tilcdc: Check drm_fb_cma_get_gem_obj() return value
Date: Tue, 5 Mar 2019 22:02:21 +0200	[thread overview]
Message-ID: <20190305200221.GJ14928@pendragon.ideasonboard.com> (raw)
In-Reply-To: <ffc0f3c0-cc40-6a4a-fe49-ba200e0fc271@ti.com>

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@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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2019-03-05 20:02 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-28 11:18 [tiL4.19-AD PATCH 0/2] drm/tilcdc: a cleanup and a fix Jyri Sarha
2019-02-28 11:18 ` [tiL4.19-AD PATCH 1/2] drm/tilcdc: Check drm_fb_cma_get_gem_obj() return value Jyri Sarha
2019-03-05 15:43   ` Laurent Pinchart
2019-03-05 19:42     ` Jyri Sarha
2019-03-05 20:02       ` Laurent Pinchart [this message]
2019-02-28 11:18 ` [tiL4.19-AD PATCH 2/2] drm/tilcdc: Remove obsolete crtc_mode_valid() hack Jyri Sarha
2019-03-05 19:21   ` Laurent Pinchart
2019-03-13 17:30     ` Jyri Sarha

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190305200221.GJ14928@pendragon.ideasonboard.com \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jsarha@ti.com \
    --cc=peter.ujfalusi@ti.com \
    --cc=tomi.valkeinen@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).