All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: intel-gfx@lists.freedesktop.org
Cc: Hans de Goede <hdegoede@redhat.com>,
	Rob Clark <rclark@redhat.com>, Adam Jackson <ajax@redhat.com>
Subject: [PATCH xf86-video-intel] Fix fd (and mem) leak when intel_scrn_create fails
Date: Fri, 22 Apr 2016 20:04:33 +0200	[thread overview]
Message-ID: <1461348273-25895-1-git-send-email-hdegoede@redhat.com> (raw)

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

             reply	other threads:[~2016-04-22 18:04 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-22 18:04 Hans de Goede [this message]
2016-04-25  8:38 ` [PATCH xf86-video-intel] Fix fd (and mem) leak when intel_scrn_create fails Timo Aaltonen
2016-05-12 17:57 Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1461348273-25895-1-git-send-email-hdegoede@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=ajax@redhat.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=rclark@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.