render/allocator: re-open GBM FD

Using the same DRM file description for the DRM backend and for the
GBM allocator will result in GEM handle ref'counting issues [1].
Re-open the DRM FD to fix these issues.

[1]: https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/110

3 files changed, 42 insertions, 32 deletions

diff --git a/render/allocator/allocator.c b/render/allocator/allocator.c
index 8664202f..affc928e 100644
--- a/render/allocator/allocator.c
+++ b/render/allocator/allocator.c
@@ -1,5 +1,8 @@
+#define _POSIX_C_SOURCE 200809L
 #include <assert.h>
+#include <fcntl.h>
 #include <stdlib.h>
+#include <unistd.h>
 #include <wlr/util/log.h>
 #include <xf86drm.h>
 #include "backend/backend.h"
@@ -17,6 +20,26 @@ void wlr_allocator_init(struct wlr_allocator *alloc,
 	wl_signal_init(&alloc->events.destroy);
 }
 
+/* Re-open the DRM node to avoid GEM handle ref'counting issues. See:
+ * https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/110
+ * TODO: don't assume we have the permission to just open the DRM node,
+ * find another way to re-open it.
+ */
+static int reopen_drm_node(int drm_fd) {
+	char *name = drmGetDeviceNameFromFd2(drm_fd);
+	if (name == NULL) {
+		wlr_log(WLR_ERROR, "drmGetDeviceNameFromFd2 failed");
+		return -1;
+	}
+
+	int new_fd = open(name, O_RDWR | O_CLOEXEC);
+	if (new_fd < 0) {
+		wlr_log_errno(WLR_ERROR, "Failed to open DRM node '%s'", name);
+	}
+	free(name);
+	return new_fd;
+}
+
 struct wlr_allocator *allocator_autocreate_with_drm_fd(
 		struct wlr_backend *backend, struct wlr_renderer *renderer,
 		int drm_fd) {
@@ -26,11 +49,16 @@ struct wlr_allocator *allocator_autocreate_with_drm_fd(
 	struct wlr_allocator *alloc = NULL;
 	uint32_t gbm_caps = WLR_BUFFER_CAP_DMABUF;
 	if ((backend_caps & gbm_caps) && (renderer_caps & gbm_caps)
-			&& drm_fd != -1) {
+			&& drm_fd >= 0) {
 		wlr_log(WLR_DEBUG, "Trying to create gbm allocator");
-		if ((alloc = wlr_gbm_allocator_create(drm_fd)) != NULL) {
+		int gbm_fd = reopen_drm_node(drm_fd);
+		if (gbm_fd < 0) {
+			return NULL;
+		}
+		if ((alloc = wlr_gbm_allocator_create(gbm_fd)) != NULL) {
 			return alloc;
 		}
+		close(gbm_fd);
 		wlr_log(WLR_DEBUG, "Failed to create gbm allocator");
 	}
 
@@ -45,11 +73,16 @@ struct wlr_allocator *allocator_autocreate_with_drm_fd(
 
 	uint32_t drm_caps = WLR_BUFFER_CAP_DMABUF | WLR_BUFFER_CAP_DATA_PTR;
 	if ((backend_caps & drm_caps) && (renderer_caps & drm_caps)
-			&& drm_fd != -1) {
+			&& drm_fd >= 0 && drmIsMaster(drm_fd)) {
 		wlr_log(WLR_DEBUG, "Trying to create drm dumb allocator");
-		if ((alloc = wlr_drm_dumb_allocator_create(drm_fd)) != NULL) {
+		int dumb_fd = reopen_drm_node(drm_fd);
+		if (dumb_fd < 0) {
+			return NULL;
+		}
+		if ((alloc = wlr_drm_dumb_allocator_create(dumb_fd)) != NULL) {
 			return alloc;
 		}
+		close(dumb_fd);
 		wlr_log(WLR_DEBUG, "Failed to create drm dumb allocator");
 	}
 
diff --git a/render/allocator/drm_dumb.c b/render/allocator/drm_dumb.c
index a47df624..ea982a03 100644
--- a/render/allocator/drm_dumb.c
+++ b/render/allocator/drm_dumb.c
@@ -199,41 +199,25 @@ static const struct wlr_allocator_interface allocator_impl = {
 	.destroy = allocator_destroy,
 };
 
-struct wlr_allocator *wlr_drm_dumb_allocator_create(int fd) {
-	if (!drmIsMaster(fd)) {
-		wlr_log(WLR_ERROR, "Cannot use DRM dumb buffers with non-master DRM FD");
-		return NULL;
-	}
-
-	/* Re-open the DRM node to avoid GEM handle ref'counting issues. See:
-	 * https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/110
-	 * TODO: don't assume we have the permission to just open the DRM node,
-	 * find another way to re-open it.
-	 */
-	char *path = drmGetDeviceNameFromFd2(fd);
-	int drm_fd = open(path, O_RDWR | O_CLOEXEC);
-	if (drm_fd < 0) {
-		wlr_log_errno(WLR_ERROR, "Failed to open DRM node %s", path);
-		free(path);
+struct wlr_allocator *wlr_drm_dumb_allocator_create(int drm_fd) {
+	if (drmGetNodeTypeFromFd(drm_fd) != DRM_NODE_PRIMARY) {
+		wlr_log(WLR_ERROR, "Cannot use DRM dumb buffers with non-primary DRM FD");
 		return NULL;
 	}
 
 	uint64_t has_dumb = 0;
 	if (drmGetCap(drm_fd, DRM_CAP_DUMB_BUFFER, &has_dumb) < 0) {
 		wlr_log(WLR_ERROR, "Failed to get DRM capabilities");
-		free(path);
 		return NULL;
 	}
 
 	if (has_dumb == 0) {
 		wlr_log(WLR_ERROR, "DRM dumb buffers not supported");
-		free(path);
 		return NULL;
 	}
 
 	struct wlr_drm_dumb_allocator *alloc = calloc(1, sizeof(*alloc));
 	if (alloc == NULL) {
-		free(path);
 		return NULL;
 	}
 	wlr_allocator_init(&alloc->base, &allocator_impl,
@@ -242,7 +226,6 @@ struct wlr_allocator *wlr_drm_dumb_allocator_create(int fd) {
 	alloc->drm_fd = drm_fd;
 	wl_list_init(&alloc->buffers);
 
-	wlr_log(WLR_DEBUG, "Created DRM dumb allocator with node %s", path);
-	free(path);
+	wlr_log(WLR_DEBUG, "Created DRM dumb allocator");
 	return &alloc->base;
 }
diff --git a/render/allocator/gbm.c b/render/allocator/gbm.c
index ace7ca68..0092afd6 100644
--- a/render/allocator/gbm.c
+++ b/render/allocator/gbm.c
@@ -163,13 +163,7 @@ static struct wlr_gbm_allocator *get_gbm_alloc_from_alloc(
 	return (struct wlr_gbm_allocator *)alloc;
 }
 
-struct wlr_allocator *wlr_gbm_allocator_create(int drm_fd) {
-	int fd = fcntl(drm_fd, F_DUPFD_CLOEXEC, 0);
-	if (fd < 0) {
-		wlr_log(WLR_ERROR, "fcntl(F_DUPFD_CLOEXEC) failed");
-		return NULL;
-	}
-
+struct wlr_allocator *wlr_gbm_allocator_create(int fd) {
 	uint64_t cap;
 	if (drmGetCap(fd, DRM_CAP_PRIME, &cap) ||
 			!(cap & DRM_PRIME_CAP_EXPORT)) {