intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH xf86-video-intel] Fix fd (and mem) leak when intel_scrn_create fails
@ 2016-04-22 18:04 Hans de Goede
  2016-04-25  8:38 ` Timo Aaltonen
  0 siblings, 1 reply; 3+ messages in thread
From: Hans de Goede @ 2016-04-22 18:04 UTC (permalink / raw)
  To: intel-gfx; +Cc: Hans de Goede, Rob Clark, Adam Jackson

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 <hdegoede@redhat.com>
---
 src/intel_device.c | 17 +++++++++++++++++
 src/intel_driver.h |  1 +
 src/intel_module.c | 19 ++++++++++++++++---
 3 files changed, 34 insertions(+), 3 deletions(-)

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
 
-- 
2.7.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH xf86-video-intel] Fix fd (and mem) leak when intel_scrn_create fails
  2016-04-22 18:04 [PATCH xf86-video-intel] Fix fd (and mem) leak when intel_scrn_create fails Hans de Goede
@ 2016-04-25  8:38 ` Timo Aaltonen
  0 siblings, 0 replies; 3+ messages in thread
From: Timo Aaltonen @ 2016-04-25  8:38 UTC (permalink / raw)
  To: Hans de Goede, intel-gfx; +Cc: Rob Clark, Adam Jackson

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 <hdegoede@redhat.com>

Thanks, fixed the fallback here too.


-- 
t
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH xf86-video-intel] Fix fd (and mem) leak when intel_scrn_create fails
@ 2016-05-12 17:57 Hans de Goede
  0 siblings, 0 replies; 3+ messages in thread
From: Hans de Goede @ 2016-05-12 17:57 UTC (permalink / raw)
  To: Chris Wilson; +Cc: Hans de Goede, intel-gfx

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 <hdegoede@redhat.com>
---
 src/intel_device.c | 17 +++++++++++++++++
 src/intel_driver.h |  1 +
 src/intel_module.c | 19 ++++++++++++++++---
 3 files changed, 34 insertions(+), 3 deletions(-)

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
 
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-05-12 17:58 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-22 18:04 [PATCH xf86-video-intel] Fix fd (and mem) leak when intel_scrn_create fails Hans de Goede
2016-04-25  8:38 ` Timo Aaltonen
  -- strict thread matches above, loose matches on Subject: below --
2016-05-12 17:57 Hans de Goede

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).