modes: Retry GETCONNECTOR if a hotplug event occurs between the two ioctls

Commit Message

Peter Clifton Jan. 6, 2010, 9:11 p.m. UTC

None

Patch

From 4339615ba4a5fb393301c8b5ad5684a88b95cd69 Mon Sep 17 00:00:00 2001
From: Peter Clifton <[e-mail]>
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 <[e-mail]>
---
 xf86drmMode.c |   58 ++++++++++++++++++++++++++++++++++++++++++--------------
 1 files changed, 43 insertions(+), 15 deletions(-)

diff --git a/xf86drmMode.c b/xf86drmMode.c
index cfdee4a..f330e6f 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));
-- 
1.6.5