[bug report] drm: Use atomic state for FB in legacy ioctls

[Daniel Vetter]

On Fri, Jan 13, 2017 at 03:12:07PM +0000, Daniel Stone wrote:
> Hi Dan,
> 
> On 13 January 2017 at 12:56, Dan Carpenter <dan.carpenter at oracle.com> 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