On Fri, Jan 13, 2017 at 03:12:07PM +, Daniel Stone wrote:
> Hi Dan,
>
> On 13 January 2017 at 12:56, Dan Carpenter wrote:
> > drivers/gpu/drm/drm_crtc.c:392 drm_mode_getcrtc()
> > error: we previously assumed 'crtc->primary->state' could be null
> > (see line 384)
> >
> > drivers/gpu/drm/drm_crtc.c
> >383
> >384 if (crtc->primary->state && crtc->primary->state->fb)
> >
> > New check for NULL.
> >
> >385 crtc_resp->fb_id =
> > crtc->primary->state->fb->base.id;
> >386 else if (!crtc->primary->state && crtc->primary->fb)
> >387 crtc_resp->fb_id = crtc->primary->fb->base.id;
> >388 else
> >389 crtc_resp->fb_id = 0;
> >390
> >391 if (crtc->state) {
> >392 crtc_resp->x = crtc->primary->state->src_x >> 16;
> >^^^
> > Old unchecked dereference. It's possible that non-NULL "crtc->state"
> > implies a non-NULL "crtc->primary->state", but I didn't spot the
> > relationship immediately.
>
> Thanks for this. I believe this is indeed an invariant; Dan Vetter
> could confirm, if he's managed to find internet in Hobart. Assuming
> this is the case, what's the best way to communicate this to smatch;
> would that be through a BUG_ON or similar?
Yeah, we assume that if a driver uses state stuff, it's used everywhere.
But I'm not sure anymore how much that holds true for a driver
transitioning to atomic, so adding a BUG_ON to tell smatch or extend the
if check to also check for crtc->primary->state (it wont make a diffrence
for atomic drivers) if the BUG_ON doesn't clue in smatch enough.
Can you pls submit the right patch, since I don't have a working smatch
setup here?
-Daniel
--
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
