On Thu, Nov 27, 2014 at 10:55:02AM +0100, Daniel Vetter wrote:
> On Thu, Nov 27, 2014 at 09:41:13AM +0300, Dan Carpenter wrote:
> > Hello Rob Clark,
> >
> > The patch 1ed2f34b4cc0: "drm/atomic: track bitmask of planes attached
> > to crtc" from Nov 21, 2014, leads to the following static checker
> > warning:
> >
> > drivers/gpu/drm/drm_atomic.c:368 drm_atomic_set_crtc_for_plane()
> > error: 'plane_state' dereferencing possible ERR_PTR()
>
> Hm yeah that shouldn't ever happen when callers use this correctly. But a
> WARN_ON would be good I guess. I'll add it.
>
It could fail because of allocation failures. But maybe this is a boot
time thing? Normally dereferencing an ERR_PTR() is easy enough to debug
and static checkers just ignore WARN_ONs. I am ambivalent.
> >
> > drivers/gpu/drm/drm_atomic.c
> > 360 int
> > 361 drm_atomic_set_crtc_for_plane(struct drm_atomic_state *state,
> > 362 struct drm_plane *plane, struct drm_crtc *crtc)
> > 363 {
> > 364 struct drm_plane_state *plane_state =
> > ^^^^^^^^^^^^^
> > 365 drm_atomic_get_plane_state(state, plane);
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > 366 struct drm_crtc_state *crtc_state;
> > 367
> > 368 if (plane_state->crtc) {
> > ^^^^^^^^^^^^^^^^^
> > Missing IS_ERR() check.
> >
> > Also drm_atomic_get_plane_state() has poor error handling. In
> > drm_atomic_get_plane_state(), if the call to drm_atomic_get_plane_state()
> > fails then it leaks memory.
>
> Where does it leak memory exactly?
drivers/gpu/drm/drm_atomic.c
249
250 plane_state = plane->funcs->atomic_duplicate_state(plane);
This is a kmemdup().
251 if (!plane_state)
252 return ERR_PTR(-ENOMEM);
253
254 state->plane_states[index] = plane_state;
255 state->planes[index] = plane;
256 plane_state->state = state;
257
258 DRM_DEBUG_KMS("Added [PLANE:%d] %p state to %p\n",
259 plane->base.id, plane_state, state);
260
261 if (plane_state->crtc) {
262 struct drm_crtc_state *crtc_state;
263
264 crtc_state = drm_atomic_get_crtc_state(state,
265 plane_state->crtc);
266 if (IS_ERR(crtc_state))
267 return ERR_CAST(crtc_state);
We leak if we return here.
268 }
269
270 return plane_state;
regards,
dan carpenter
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
