From c0eada20198430109f5072d13b51cf4554a1dec3 Mon Sep 17 00:00:00 2001 From: Eric Curtin 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 | 222 ++++++++++++++--------------- 1 file changed, 109 insertions(+), 113 deletions(-) diff --git a/src/video/kmsdrm/SDL_kmsdrmvideo.c b/src/video/kmsdrm/SDL_kmsdrmvideo.c index fd3a3cd63..07a15ae67 100644 --- a/src/video/kmsdrm/SDL_kmsdrmvideo.c +++ b/src/video/kmsdrm/SDL_kmsdrmvideo.c @@ -53,131 +53,112 @@ #include #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) -{ - SDL_bool available = SDL_FALSE; - char device[512]; - int drm_fd; - int i; - - 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) { - continue; - } - } - - available = SDL_TRUE; - break; - } - - KMSDRM_drmModeFreeConnector(conn); - } - } - KMSDRM_drmModeFreeResources(resources); - } - 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++; - } - } - closedir(folder); - } - - return devcount; -} - static int get_driindex(void) { - const int devcount = get_dricount(); + int available = -ENOENT; + char device[sizeof(kmsdrm_dri_cardpath)]; + int drm_fd; int i; + int devindex = -1; + DIR *folder; - for (i = 0; i < devcount; i++) { - if (check_modestting(i)) { - return i; + 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; + } + + 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_drmModeFreeResources(resources); + } + SDL_KMSDRM_UnloadSymbols(); + } + close(drm_fd); + } + + SDL_LogDebug(SDL_LOG_CATEGORY_VIDEO, + "Failed to open KMSDRM device %s, errno: %d\n", device, + errno); } } - return -ENOENT; + closedir(folder); + + 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);