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 --- 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);
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
dri-devel@lists.freedesktop.org