From c0eada20198430109f5072d13b51cf4554a1dec3 Mon Sep 17 00:00:00 2001
From: Eric Curtin <[EMAIL REDACTED]>
Date: Wed, 6 Jul 2022 17:00:16 +0100
Subject: [PATCH] Fix assumption that DRI_DEVNAME begins at 0 (#5865)
* Fix assumption that DRI_DEVNAME begins at 0
The existing logic of the code was to count every possible entry in
KMSDRM_DRI_PATH. After this a for loop would start trying to open
filename0, filename1, filename2, etc. In recent Linux kernels (say
5.18) with simpledrm, the lowest KMSDRM_DRI_DEVNAME is often
/dev/dri/card1, rather than /dev/dri/card0, causing the code to fail
once /dev/dri/card0 has failed to open. Running:
modprobe foodrm && modprobe bardrm && rmmod foodrm
before you try to run an application with SDL KMSDRM would have also
made this fail.
* Various changes from review
- Removed newline and period from SDL error
- Explicitely compare memcmp to zero (also changed to SDL_memcmp)
- Changed memcpy to strncpy
- Less aggressive line wrapping
* Various changes from review
- strncpy to SDL_strlcpy
- removed size hardcodings for KMSDRM_DRI_PATHSIZE and
KMSDRM_DRI_DEVNAMESIZE
- made all KMSDRM_DRI defines, run-time variables to reduce bugs caused
by these defines being more build-time on Linux and more run-rime on
OpenBSD
- renamed openbsd69orgreater variable to moderndri
- altered comment from "if on OpenBSD" to add difference in 6.9
* Various changes from review
- Use max size of destination, rather than max size of source
- Less hardcodings
---
src/video/kmsdrm/SDL_kmsdrmvideo.c | 204 ++++++++++++++---------------
1 file changed, 100 insertions(+), 104 deletions(-)
diff --git a/src/video/kmsdrm/SDL_kmsdrmvideo.c b/src/video/kmsdrm/SDL_kmsdrmvideo.c
index fd3a3cd63e0..07a15ae6718 100644
--- a/src/video/kmsdrm/SDL_kmsdrmvideo.c
+++ b/src/video/kmsdrm/SDL_kmsdrmvideo.c
@@ -53,131 +53,112 @@
#include <errno.h>
#ifdef __OpenBSD__
-static SDL_bool openbsd69orgreater = SDL_FALSE;
-#define KMSDRM_DRI_PATH openbsd69orgreater ? "/dev/dri/" : "/dev/"
-#define KMSDRM_DRI_DEVFMT openbsd69orgreater ? "%scard%d" : "%sdrm%d"
-#define KMSDRM_DRI_DEVNAME openbsd69orgreater ? "card" : "drm"
-#define KMSDRM_DRI_DEVNAMESIZE openbsd69orgreater ? 4 : 3
-#define KMSDRM_DRI_CARDPATHFMT openbsd69orgreater ? "/dev/dri/card%d" : "/dev/drm%d"
+static SDL_bool moderndri = SDL_FALSE;
#else
-#define KMSDRM_DRI_PATH "/dev/dri/"
-#define KMSDRM_DRI_DEVFMT "%scard%d"
-#define KMSDRM_DRI_DEVNAME "card"
-#define KMSDRM_DRI_DEVNAMESIZE 4
-#define KMSDRM_DRI_CARDPATHFMT "/dev/dri/card%d"
+static SDL_bool moderndri = SDL_TRUE;
#endif
+static char kmsdrm_dri_path[16];
+static int kmsdrm_dri_pathsize = 0;
+static char kmsdrm_dri_devname[8];
+static int kmsdrm_dri_devnamesize = 0;
+static char kmsdrm_dri_cardpath[32];
+
#ifndef EGL_PLATFORM_GBM_MESA
#define EGL_PLATFORM_GBM_MESA 0x31D7
#endif
static int
-check_modestting(int devindex)
+get_driindex(void)
{
- SDL_bool available = SDL_FALSE;
- char device[512];
+ int available = -ENOENT;
+ char device[sizeof(kmsdrm_dri_cardpath)];
int drm_fd;
int i;
+ int devindex = -1;
+ DIR *folder;
- SDL_snprintf(device, sizeof (device), KMSDRM_DRI_DEVFMT, KMSDRM_DRI_PATH, devindex);
-
- drm_fd = open(device, O_RDWR | O_CLOEXEC);
- if (drm_fd >= 0) {
- if (SDL_KMSDRM_LoadSymbols()) {
- drmModeRes *resources = KMSDRM_drmModeGetResources(drm_fd);
- if (resources) {
- SDL_LogDebug(SDL_LOG_CATEGORY_VIDEO,
- KMSDRM_DRI_DEVFMT
- " connector, encoder and CRTC counts are: %d %d %d",
- KMSDRM_DRI_PATH, devindex,
- resources->count_connectors, resources->count_encoders,
- resources->count_crtcs);
-
- if (resources->count_connectors > 0
- && resources->count_encoders > 0
- && resources->count_crtcs > 0)
- {
- available = SDL_FALSE;
- for (i = 0; i < resources->count_connectors; i++) {
- drmModeConnector *conn = KMSDRM_drmModeGetConnector(drm_fd,
- resources->connectors[i]);
-
- if (!conn) {
- continue;
- }
-
- if (conn->connection == DRM_MODE_CONNECTED && conn->count_modes) {
- if (SDL_GetHintBoolean(SDL_HINT_KMSDRM_REQUIRE_DRM_MASTER, SDL_TRUE)) {
- /* Skip this device if we can't obtain DRM master */
- KMSDRM_drmSetMaster(drm_fd);
- if (KMSDRM_drmAuthMagic(drm_fd, 0) == -EACCES) {
+ SDL_strlcpy(device, kmsdrm_dri_path, sizeof(device));
+ folder = opendir(device);
+ if (!folder) {
+ SDL_SetError("Failed to open directory '%s'", device);
+ return -ENOENT;
+ }
+
+ SDL_strlcpy(device + kmsdrm_dri_pathsize, kmsdrm_dri_devname,
+ sizeof(device) - kmsdrm_dri_devnamesize);
+ for (struct dirent *res; (res = readdir(folder));) {
+ if (SDL_memcmp(res->d_name, kmsdrm_dri_devname,
+ kmsdrm_dri_devnamesize) == 0) {
+ SDL_strlcpy(device + kmsdrm_dri_pathsize + kmsdrm_dri_devnamesize,
+ res->d_name + kmsdrm_dri_devnamesize,
+ sizeof(device) - kmsdrm_dri_pathsize -
+ kmsdrm_dri_devnamesize);
+
+ drm_fd = open(device, O_RDWR | O_CLOEXEC);
+ if (drm_fd >= 0) {
+ devindex = SDL_atoi(device + kmsdrm_dri_pathsize +
+ kmsdrm_dri_devnamesize);
+ if (SDL_KMSDRM_LoadSymbols()) {
+ drmModeRes *resources = KMSDRM_drmModeGetResources(drm_fd);
+ if (resources) {
+ SDL_LogDebug(SDL_LOG_CATEGORY_VIDEO,
+ "%s%d connector, encoder and CRTC counts are: %d %d %d",
+ kmsdrm_dri_cardpath, devindex,
+ resources->count_connectors,
+ resources->count_encoders,
+ resources->count_crtcs);
+
+ if (resources->count_connectors > 0 &&
+ resources->count_encoders > 0 &&
+ resources->count_crtcs > 0) {
+ available = -ENOENT;
+ for (i = 0; i < resources->count_connectors; i++) {
+ drmModeConnector *conn =
+ KMSDRM_drmModeGetConnector(
+ drm_fd, resources->connectors[i]);
+
+ if (!conn) {
continue;
}
- }
- available = SDL_TRUE;
- break;
- }
+ if (conn->connection == DRM_MODE_CONNECTED &&
+ conn->count_modes) {
+ if (SDL_GetHintBoolean(
+ SDL_HINT_KMSDRM_REQUIRE_DRM_MASTER,
+ SDL_TRUE)) {
+ /* Skip this device if we can't obtain
+ * DRM master */
+ KMSDRM_drmSetMaster(drm_fd);
+ if (KMSDRM_drmAuthMagic(drm_fd, 0) ==
+ -EACCES) {
+ continue;
+ }
+ }
+
+ available = devindex;
+ break;
+ }
- KMSDRM_drmModeFreeConnector(conn);
+ KMSDRM_drmModeFreeConnector(conn);
+ }
+ }
+ KMSDRM_drmModeFreeResources(resources);
}
+ SDL_KMSDRM_UnloadSymbols();
}
- KMSDRM_drmModeFreeResources(resources);
+ close(drm_fd);
}
- SDL_KMSDRM_UnloadSymbols();
- }
- close(drm_fd);
- }
-
- return available;
-}
-
-static int get_dricount(void)
-{
- int devcount = 0;
- struct dirent *res;
- struct stat sb;
- DIR *folder;
-
- if (!(stat(KMSDRM_DRI_PATH, &sb) == 0
- && S_ISDIR(sb.st_mode))) {
- /*printf("The path %s cannot be opened or is not available\n", KMSDRM_DRI_PATH);*/
- return 0;
- }
- if (access(KMSDRM_DRI_PATH, F_OK) == -1) {
- /*printf("The path %s cannot be opened\n", KMSDRM_DRI_PATH);*/
- return 0;
- }
-
- folder = opendir(KMSDRM_DRI_PATH);
- if (folder) {
- while ((res = readdir(folder))) {
- int len = SDL_strlen(res->d_name);
- if (len > KMSDRM_DRI_DEVNAMESIZE && SDL_strncmp(res->d_name,
- KMSDRM_DRI_DEVNAME, KMSDRM_DRI_DEVNAMESIZE) == 0) {
- devcount++;
- }
+ SDL_LogDebug(SDL_LOG_CATEGORY_VIDEO,
+ "Failed to open KMSDRM device %s, errno: %d\n", device,
+ errno);
}
- closedir(folder);
}
- return devcount;
-}
-
-static int
-get_driindex(void)
-{
- const int devcount = get_dricount();
- int i;
-
- for (i = 0; i < devcount; i++) {
- if (check_modestting(i)) {
- return i;
- }
- }
+ closedir(folder);
- return -ENOENT;
+ return available;
}
static int
@@ -193,10 +174,24 @@ KMSDRM_Available(void)
if (!(uname(&nameofsystem) < 0)) {
releaseversion = SDL_atof(nameofsystem.release);
if (releaseversion >= 6.9) {
- openbsd69orgreater = SDL_TRUE;
+ moderndri = SDL_TRUE;
}
}
#endif
+
+ if (moderndri) {
+ SDL_strlcpy(kmsdrm_dri_path, "/dev/dri/", sizeof(kmsdrm_dri_path));
+ SDL_strlcpy(kmsdrm_dri_devname, "card", sizeof(kmsdrm_dri_devname));
+ } else {
+ SDL_strlcpy(kmsdrm_dri_path, "/dev/", sizeof(kmsdrm_dri_path));
+ SDL_strlcpy(kmsdrm_dri_devname, "drm", sizeof(kmsdrm_dri_devname));
+ }
+
+ kmsdrm_dri_pathsize = SDL_strlen(kmsdrm_dri_path);
+ kmsdrm_dri_devnamesize = SDL_strlen(kmsdrm_dri_devname);
+ SDL_snprintf(kmsdrm_dri_cardpath, sizeof(kmsdrm_dri_cardpath), "%s%s",
+ kmsdrm_dri_path, kmsdrm_dri_devname);
+
ret = get_driindex();
if (ret >= 0)
return 1;
@@ -732,8 +727,9 @@ KMSDRM_InitDisplays (_THIS) {
int ret = 0;
int i;
- /* Open /dev/dri/cardNN (/dev/drmN if on OpenBSD) */
- SDL_snprintf(viddata->devpath, sizeof(viddata->devpath), KMSDRM_DRI_CARDPATHFMT, viddata->devindex);
+ /* Open /dev/dri/cardNN (/dev/drmN if on OpenBSD version less than 6.9) */
+ SDL_snprintf(viddata->devpath, sizeof(viddata->devpath), "%s%d",
+ kmsdrm_dri_cardpath, viddata->devindex);
SDL_LogDebug(SDL_LOG_CATEGORY_VIDEO, "Opening device %s", viddata->devpath);
viddata->drm_fd = open(viddata->devpath, O_RDWR | O_CLOEXEC);