From 04f90a44709a48fb932ea954011cb551659bf246 Mon Sep 17 00:00:00 2001 From: Peter Clifton Date: Wed, 6 Jan 2010 20:44:11 +0000 Subject: [PATCH] modes: Retry GETCONNECTOR if a hotplug event occurs between the two ioctls If the available modes changes between the two GETCONNECTOR ioctls, that caused the kernel to skip filling one array and led to a crash (as the size of the allocated and initialised block of memory differed from the reported size, and might be NULL if no modes were present at first). This bug manifest its self on my machine due to spurious false positive detections of a connected TV-out. Fixes: http://bugs.freedesktop.org/show_bug.cgi?id=25912 Crash whilst probing modes Based upon the similar fixes for the GETRESOURCES ioctls by Chris Wilson, in the following commits: commit e6c136ca7a4c54457b48be1aec2be024b3e4a28d commit 85fb3e55fdb7af9b5f59c1ec0f15d1950e601b05 commit d1308f4fe7f94aae51ca9f70947aea8e09597f37 Signed-off-by: Peter Clifton Signed-off-by: Chris Wilson --- xf86drmMode.c | 58 ++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 43 insertions(+), 15 deletions(-) diff --git a/xf86drmMode.c b/xf86drmMode.c index cfdee4a9..f330e6f2 100644 --- a/xf86drmMode.c +++ b/xf86drmMode.c @@ -410,37 +410,57 @@ drmModeEncoderPtr drmModeGetEncoder(int fd, uint32_t encoder_id) drmModeConnectorPtr drmModeGetConnector(int fd, uint32_t connector_id) { - struct drm_mode_get_connector conn; + struct drm_mode_get_connector conn, counts; drmModeConnectorPtr r = NULL; +retry: + memset(&conn, 0, sizeof(struct drm_mode_get_connector)); conn.connector_id = connector_id; - conn.connector_type_id = 0; - conn.connector_type = 0; - conn.count_modes = 0; - conn.modes_ptr = 0; - conn.count_props = 0; - conn.props_ptr = 0; - conn.prop_values_ptr = 0; - conn.count_encoders = 0; - conn.encoders_ptr = 0; if (drmIoctl(fd, DRM_IOCTL_MODE_GETCONNECTOR, &conn)) return 0; + counts = conn; + if (conn.count_props) { conn.props_ptr = VOID2U64(drmMalloc(conn.count_props*sizeof(uint32_t))); + if (!conn.props_ptr) + goto err_allocs; conn.prop_values_ptr = VOID2U64(drmMalloc(conn.count_props*sizeof(uint64_t))); + if (!conn.prop_values_ptr) + goto err_allocs; } - if (conn.count_modes) + if (conn.count_modes) { conn.modes_ptr = VOID2U64(drmMalloc(conn.count_modes*sizeof(struct drm_mode_modeinfo))); + if (!conn.modes_ptr) + goto err_allocs; + } - if (conn.count_encoders) + if (conn.count_encoders) { conn.encoders_ptr = VOID2U64(drmMalloc(conn.count_encoders*sizeof(uint32_t))); + if (!conn.encoders_ptr) + goto err_allocs; + } if (drmIoctl(fd, DRM_IOCTL_MODE_GETCONNECTOR, &conn)) goto err_allocs; + /* The number of available connectors and etc may have changed with a + * hotplug event in between the ioctls, in which case the field is + * silently ignored by the kernel. + */ + if (counts.count_props < conn.count_props || + counts.count_modes < conn.count_modes || + counts.count_encoders < conn.count_encoders) { + drmFree(U642VOID(conn.props_ptr)); + drmFree(U642VOID(conn.prop_values_ptr)); + drmFree(U642VOID(conn.modes_ptr)); + drmFree(U642VOID(conn.encoders_ptr)); + + goto retry; + } + if(!(r = drmMalloc(sizeof(*r)))) { goto err_allocs; } @@ -453,7 +473,6 @@ drmModeConnectorPtr drmModeGetConnector(int fd, uint32_t connector_id) /* convert subpixel from kernel to userspace */ r->subpixel = conn.subpixel + 1; r->count_modes = conn.count_modes; - /* TODO we should test if these alloc & cpy fails. */ r->count_props = conn.count_props; r->props = drmAllocCpy(U642VOID(conn.props_ptr), conn.count_props, sizeof(uint32_t)); r->prop_values = drmAllocCpy(U642VOID(conn.prop_values_ptr), conn.count_props, sizeof(uint64_t)); @@ -463,8 +482,17 @@ drmModeConnectorPtr drmModeGetConnector(int fd, uint32_t connector_id) r->connector_type = conn.connector_type; r->connector_type_id = conn.connector_type_id; - if (!r->props || !r->prop_values || !r->modes || !r->encoders) - goto err_allocs; + if ((r->count_props && !r->props) || + (r->count_props && !r->prop_values) || + (r->count_modes && !r->modes) || + (r->count_encoders && !r->encoders)) { + drmFree(r->props); + drmFree(r->prop_values); + drmFree(r->modes); + drmFree(r->encoders); + drmFree(r); + r = 0; + } err_allocs: drmFree(U642VOID(conn.prop_values_ptr));