[xf86-video-intel] Fix fd (and mem) leak when intel_scrn_create fails

Commit Message

Hans de Goede April 22, 2016, 6:04 p.m. UTC

The probe functions in intel_module.c call intel_open_device() before
calling intel_scrn_create(), but if the later fails because of e.g.
an unsupported (new) pci-id they were not cleaning up the resources
claimed by intel_open_device(), esp. leaking the fd is a problem
because this breaks the fallback to the modesetting driver.

This commit fixes this by adding a intel_close_device() cleanup
function and calling that when intel_scrn_create() fails.

Signed-off-by: Hans de Goede <[e-mail]>
---
 src/intel_device.c | 17 +++++++++++++++++
 src/intel_driver.h |  1 +
 src/intel_module.c | 19 ++++++++++++++++---
 3 files changed, 34 insertions(+), 3 deletions(-)

Comments

Timo Aaltonen April 25, 2016, 8:38 a.m. UTC | #1

22.04.2016, 21:04, Hans de Goede kirjoitti:
> The probe functions in intel_module.c call intel_open_device() before
> calling intel_scrn_create(), but if the later fails because of e.g.
> an unsupported (new) pci-id they were not cleaning up the resources
> claimed by intel_open_device(), esp. leaking the fd is a problem
> because this breaks the fallback to the modesetting driver.
> 
> This commit fixes this by adding a intel_close_device() cleanup
> function and calling that when intel_scrn_create() fails.
> 
> Signed-off-by: Hans de Goede <[e-mail]>

Thanks, fixed the fallback here too.

Patch

diff --git a/src/intel_device.c b/src/intel_device.c
index 54c1443..71fd851 100644
--- a/src/intel_device.c
+++ b/src/intel_device.c
@@ -643,6 +643,23 @@  err_path:
 	return -1;
 }
 
+void intel_close_device(int entity_num)
+{
+	struct intel_device *dev;
+
+	dev = xf86GetEntityPrivate(entity_num, intel_device_key)->ptr;
+
+	xf86GetEntityPrivate(entity_num, intel_device_key)->ptr = NULL;
+
+	if (dev->master_count == 0) /* Don't close server-fds */
+		close(dev->fd);
+
+	if (dev->render_node != dev->master_node)
+		free(dev->render_node);
+	free(dev->master_node);
+	free(dev);
+}
+
 int __intel_peek_fd(ScrnInfoPtr scrn)
 {
 	struct intel_device *dev;
diff --git a/src/intel_driver.h b/src/intel_driver.h
index fc9beaf..bece88a 100644
--- a/src/intel_driver.h
+++ b/src/intel_driver.h
@@ -124,6 +124,7 @@  int intel_entity_get_devid(int index);
 int intel_open_device(int entity_num,
 		      const struct pci_device *pci,
 		      struct xf86_platform_device *dev);
+void intel_close_device(int entity_num);
 int __intel_peek_fd(ScrnInfoPtr scrn);
 struct intel_device *intel_get_device(ScrnInfoPtr scrn, int *fd);
 int intel_has_render_node(struct intel_device *dev);
diff --git a/src/intel_module.c b/src/intel_module.c
index 5979cb9..8dc1cc7 100644
--- a/src/intel_module.c
+++ b/src/intel_module.c
@@ -638,6 +638,8 @@  static Bool intel_pci_probe(DriverPtr		driver,
 			    struct pci_device	*pci,
 			    intptr_t		match_data)
 {
+	Bool result;
+
 	if (intel_open_device(entity_num, pci, NULL) == -1) {
 #if UMS
 		switch (pci->device_id) {
@@ -655,7 +657,11 @@  static Bool intel_pci_probe(DriverPtr		driver,
 #endif
 	}
 
-	return intel_scrn_create(driver, entity_num, match_data, 0);
+	result = intel_scrn_create(driver, entity_num, match_data, 0);
+	if (!result)
+		intel_close_device(entity_num);
+
+	return result;
 }
 
 #ifdef XSERVER_PLATFORM_BUS
@@ -666,6 +672,7 @@  intel_platform_probe(DriverPtr driver,
 		     intptr_t match_data)
 {
 	unsigned scrn_flags = 0;
+	Bool result;
 
 	if (intel_open_device(entity_num, dev->pdev, dev) == -1)
 		return FALSE;
@@ -677,10 +684,16 @@  intel_platform_probe(DriverPtr driver,
 	}
 
 	/* if we get any flags we don't understand fail to probe for now */
-	if (flags)
+	if (flags) {
+		intel_close_device(entity_num);
 		return FALSE;
+	}
+
+	result = intel_scrn_create(driver, entity_num, match_data, scrn_flags);
+	if (!result)
+		intel_close_device(entity_num);
 
-	return intel_scrn_create(driver, entity_num, match_data, scrn_flags);
+	return result;
 }
 #endif