| Message ID | 1418207929-22363-1-git-send-email-thierry.reding@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Dec 10, 2014 at 11:38:49AM +0100, Thierry Reding wrote: > From: Thierry Reding <treding@nvidia.com> > > Drivers where the DRM objects have a lifetime that extends beyond that > of the DRM device need to zero out the DRM object memory to void stale > data such as properties. The DRM core code expects to operate on newly > allocated and zeroed out objects and will behave unexpectedly, such as > add duplicate properties, otherwise. > > Signed-off-by: Thierry Reding <treding@nvidia.com> Yeah, makes sense imo to do in the core. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Also stuffed into drm-misc and I think I'll throw that at Dave today or tomorrow. -Daniel > --- > drivers/gpu/drm/drm_crtc.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 4c9aaf8e6176..4e35d2f74e0f 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -725,6 +725,8 @@ void drm_crtc_cleanup(struct drm_crtc *crtc) > WARN_ON(crtc->state && !crtc->funcs->atomic_destroy_state); > if (crtc->state && crtc->funcs->atomic_destroy_state) > crtc->funcs->atomic_destroy_state(crtc, crtc->state); > + > + memset(crtc, 0, sizeof(*crtc)); > } > EXPORT_SYMBOL(drm_crtc_cleanup); > > @@ -927,6 +929,8 @@ void drm_connector_cleanup(struct drm_connector *connector) > if (connector->state && connector->funcs->atomic_destroy_state) > connector->funcs->atomic_destroy_state(connector, > connector->state); > + > + memset(connector, 0, sizeof(*connector)); > } > EXPORT_SYMBOL(drm_connector_cleanup); > > @@ -1068,6 +1072,8 @@ void drm_bridge_cleanup(struct drm_bridge *bridge) > list_del(&bridge->head); > dev->mode_config.num_bridge--; > drm_modeset_unlock_all(dev); > + > + memset(bridge, 0, sizeof(*bridge)); > } > EXPORT_SYMBOL(drm_bridge_cleanup); > > @@ -1134,10 +1140,11 @@ void drm_encoder_cleanup(struct drm_encoder *encoder) > drm_modeset_lock_all(dev); > drm_mode_object_put(dev, &encoder->base); > kfree(encoder->name); > - encoder->name = NULL; > list_del(&encoder->head); > dev->mode_config.num_encoder--; > drm_modeset_unlock_all(dev); > + > + memset(encoder, 0, sizeof(*encoder)); > } > EXPORT_SYMBOL(drm_encoder_cleanup); > > @@ -1257,6 +1264,8 @@ void drm_plane_cleanup(struct drm_plane *plane) > WARN_ON(plane->state && !plane->funcs->atomic_destroy_state); > if (plane->state && plane->funcs->atomic_destroy_state) > plane->funcs->atomic_destroy_state(plane, plane->state); > + > + memset(plane, 0, sizeof(*plane)); > } > EXPORT_SYMBOL(drm_plane_cleanup); > > -- > 2.1.3 >
I have test this patch on my side, plane properties are no more duplicated so you can add my ack. Thanks Benjamin 2014-12-10 12:06 GMT+01:00 Daniel Vetter <daniel@ffwll.ch>: > On Wed, Dec 10, 2014 at 11:38:49AM +0100, Thierry Reding wrote: > > From: Thierry Reding <treding@nvidia.com> > > > > Drivers where the DRM objects have a lifetime that extends beyond that > > of the DRM device need to zero out the DRM object memory to void stale > > data such as properties. The DRM core code expects to operate on newly > > allocated and zeroed out objects and will behave unexpectedly, such as > > add duplicate properties, otherwise. > > > > Signed-off-by: Thierry Reding <treding@nvidia.com> > > Yeah, makes sense imo to do in the core. Reviewed-by: Daniel Vetter < > daniel.vetter@ffwll.ch> > > Also stuffed into drm-misc and I think I'll throw that at Dave today or > tomorrow. > -Daniel > > > --- > > drivers/gpu/drm/drm_crtc.c | 11 ++++++++++- > > 1 file changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > > index 4c9aaf8e6176..4e35d2f74e0f 100644 > > --- a/drivers/gpu/drm/drm_crtc.c > > +++ b/drivers/gpu/drm/drm_crtc.c > > @@ -725,6 +725,8 @@ void drm_crtc_cleanup(struct drm_crtc *crtc) > > WARN_ON(crtc->state && !crtc->funcs->atomic_destroy_state); > > if (crtc->state && crtc->funcs->atomic_destroy_state) > > crtc->funcs->atomic_destroy_state(crtc, crtc->state); > > + > > + memset(crtc, 0, sizeof(*crtc)); > > } > > EXPORT_SYMBOL(drm_crtc_cleanup); > > > > @@ -927,6 +929,8 @@ void drm_connector_cleanup(struct drm_connector > *connector) > > if (connector->state && connector->funcs->atomic_destroy_state) > > connector->funcs->atomic_destroy_state(connector, > > connector->state); > > + > > + memset(connector, 0, sizeof(*connector)); > > } > > EXPORT_SYMBOL(drm_connector_cleanup); > > > > @@ -1068,6 +1072,8 @@ void drm_bridge_cleanup(struct drm_bridge *bridge) > > list_del(&bridge->head); > > dev->mode_config.num_bridge--; > > drm_modeset_unlock_all(dev); > > + > > + memset(bridge, 0, sizeof(*bridge)); > > } > > EXPORT_SYMBOL(drm_bridge_cleanup); > > > > @@ -1134,10 +1140,11 @@ void drm_encoder_cleanup(struct drm_encoder > *encoder) > > drm_modeset_lock_all(dev); > > drm_mode_object_put(dev, &encoder->base); > > kfree(encoder->name); > > - encoder->name = NULL; > > list_del(&encoder->head); > > dev->mode_config.num_encoder--; > > drm_modeset_unlock_all(dev); > > + > > + memset(encoder, 0, sizeof(*encoder)); > > } > > EXPORT_SYMBOL(drm_encoder_cleanup); > > > > @@ -1257,6 +1264,8 @@ void drm_plane_cleanup(struct drm_plane *plane) > > WARN_ON(plane->state && !plane->funcs->atomic_destroy_state); > > if (plane->state && plane->funcs->atomic_destroy_state) > > plane->funcs->atomic_destroy_state(plane, plane->state); > > + > > + memset(plane, 0, sizeof(*plane)); > > } > > EXPORT_SYMBOL(drm_plane_cleanup); > > > > -- > > 2.1.3 > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch >
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 4c9aaf8e6176..4e35d2f74e0f 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -725,6 +725,8 @@ void drm_crtc_cleanup(struct drm_crtc *crtc) WARN_ON(crtc->state && !crtc->funcs->atomic_destroy_state); if (crtc->state && crtc->funcs->atomic_destroy_state) crtc->funcs->atomic_destroy_state(crtc, crtc->state); + + memset(crtc, 0, sizeof(*crtc)); } EXPORT_SYMBOL(drm_crtc_cleanup); @@ -927,6 +929,8 @@ void drm_connector_cleanup(struct drm_connector *connector) if (connector->state && connector->funcs->atomic_destroy_state) connector->funcs->atomic_destroy_state(connector, connector->state); + + memset(connector, 0, sizeof(*connector)); } EXPORT_SYMBOL(drm_connector_cleanup); @@ -1068,6 +1072,8 @@ void drm_bridge_cleanup(struct drm_bridge *bridge) list_del(&bridge->head); dev->mode_config.num_bridge--; drm_modeset_unlock_all(dev); + + memset(bridge, 0, sizeof(*bridge)); } EXPORT_SYMBOL(drm_bridge_cleanup); @@ -1134,10 +1140,11 @@ void drm_encoder_cleanup(struct drm_encoder *encoder) drm_modeset_lock_all(dev); drm_mode_object_put(dev, &encoder->base); kfree(encoder->name); - encoder->name = NULL; list_del(&encoder->head); dev->mode_config.num_encoder--; drm_modeset_unlock_all(dev); + + memset(encoder, 0, sizeof(*encoder)); } EXPORT_SYMBOL(drm_encoder_cleanup); @@ -1257,6 +1264,8 @@ void drm_plane_cleanup(struct drm_plane *plane) WARN_ON(plane->state && !plane->funcs->atomic_destroy_state); if (plane->state && plane->funcs->atomic_destroy_state) plane->funcs->atomic_destroy_state(plane, plane->state); + + memset(plane, 0, sizeof(*plane)); } EXPORT_SYMBOL(drm_plane_cleanup);