[xserver,1/4] os: move xf86PrivsElevated here

Ben Crocker bcrocker at redhat.com
Thu Mar 8 21:53:57 UTC 2018


From: Nicolai Hähnle <nhaehnle at gmail.com>

From: Nicolai Hähnle <nicolai.haehnle at amd.com>

Having different types of code all trying to check for elevated privileges
is a bad idea. This implementation is the most thorough one.

Signed-off-by: Nicolai Hähnle <nicolai.haehnle at amd.com>
---
 hw/xfree86/common/xf86Init.c | 59 +----------------------------------------
 include/os.h                 |  3 +++
 os/utils.c                   | 63 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+), 58 deletions(-)

diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c
index e61fe66..758b926 100644
--- a/hw/xfree86/common/xf86Init.c
+++ b/hw/xfree86/common/xf86Init.c
@@ -230,78 +230,21 @@ xf86PrintBanner(void)
     xf86ErrorFVerb(0, "Current version of pixman: %s\n",
                    pixman_version_string());
     xf86ErrorFVerb(0, "\tBefore reporting problems, check "
                    "" __VENDORDWEBSUPPORT__ "\n"
                    "\tto make sure that you have the latest version.\n");
 }
 
 Bool
 xf86PrivsElevated(void)
 {
-    static Bool privsTested = FALSE;
-    static Bool privsElevated = TRUE;
-
-    if (!privsTested) {
-#if defined(WIN32)
-        privsElevated = FALSE;
-#else
-        if ((getuid() != geteuid()) || (getgid() != getegid())) {
-            privsElevated = TRUE;
-        }
-        else {
-#if defined(HAVE_ISSETUGID)
-            privsElevated = issetugid();
-#elif defined(HAVE_GETRESUID)
-            uid_t ruid, euid, suid;
-            gid_t rgid, egid, sgid;
-
-            if ((getresuid(&ruid, &euid, &suid) == 0) &&
-                (getresgid(&rgid, &egid, &sgid) == 0)) {
-                privsElevated = (euid != suid) || (egid != sgid);
-            }
-            else {
-                printf("Failed getresuid or getresgid");
-                /* Something went wrong, make defensive assumption */
-                privsElevated = TRUE;
-            }
-#else
-            if (getuid() == 0) {
-                /* running as root: uid==euid==0 */
-                privsElevated = FALSE;
-            }
-            else {
-                /*
-                 * If there are saved ID's the process might still be privileged
-                 * even though the above test succeeded. If issetugid() and
-                 * getresgid() aren't available, test this by trying to set
-                 * euid to 0.
-                 */
-                unsigned int oldeuid;
-
-                oldeuid = geteuid();
-
-                if (seteuid(0) != 0) {
-                    privsElevated = FALSE;
-                }
-                else {
-                    if (seteuid(oldeuid) != 0) {
-                        FatalError("Failed to drop privileges.  Exiting\n");
-                    }
-                    privsElevated = TRUE;
-                }
-            }
-#endif
-        }
-#endif
-        privsTested = TRUE;
-    }
-    return privsElevated;
+    return PrivsElevated();
 }
 
 static void
 TrapSignals(void)
 {
     if (xf86Info.notrapSignals) {
         OsSignal(SIGSEGV, SIG_DFL);
         OsSignal(SIGABRT, SIG_DFL);
         OsSignal(SIGILL, SIG_DFL);
 #ifdef SIGEMT
diff --git a/include/os.h b/include/os.h
index d2c41b4..686f6d6 100644
--- a/include/os.h
+++ b/include/os.h
@@ -355,20 +355,23 @@ Fclose(void *);
 extern const char *
 Win32TempDir(void);
 
 extern int
 System(const char *cmdline);
 
 #define Fopen(a,b) fopen(a,b)
 #define Fclose(a) fclose(a)
 #endif
 
+extern _X_EXPORT Bool
+PrivsElevated(void);
+
 extern _X_EXPORT void
 CheckUserParameters(int argc, char **argv, char **envp);
 extern _X_EXPORT void
 CheckUserAuthorization(void);
 
 extern _X_EXPORT int
 AddHost(ClientPtr /*client */ ,
         int /*family */ ,
         unsigned /*length */ ,
         const void * /*pAddr */ );
diff --git a/os/utils.c b/os/utils.c
index ac55cd7..024989e 100644
--- a/os/utils.c
+++ b/os/utils.c
@@ -1717,20 +1717,83 @@ System(const char *cmdline)
 
     /* Close process and thread handles. */
     CloseHandle(pi.hProcess);
     CloseHandle(pi.hThread);
     free(cmd);
 
     return dwExitCode;
 }
 #endif
 
+Bool
+PrivsElevated(void)
+{
+    static Bool privsTested = FALSE;
+    static Bool privsElevated = TRUE;
+
+    if (!privsTested) {
+#if defined(WIN32)
+        privsElevated = FALSE;
+#else
+        if ((getuid() != geteuid()) || (getgid() != getegid())) {
+            privsElevated = TRUE;
+        }
+        else {
+#if defined(HAVE_ISSETUGID)
+            privsElevated = issetugid();
+#elif defined(HAVE_GETRESUID)
+            uid_t ruid, euid, suid;
+            gid_t rgid, egid, sgid;
+
+            if ((getresuid(&ruid, &euid, &suid) == 0) &&
+                (getresgid(&rgid, &egid, &sgid) == 0)) {
+                privsElevated = (euid != suid) || (egid != sgid);
+            }
+            else {
+                printf("Failed getresuid or getresgid");
+                /* Something went wrong, make defensive assumption */
+                privsElevated = TRUE;
+            }
+#else
+            if (getuid() == 0) {
+                /* running as root: uid==euid==0 */
+                privsElevated = FALSE;
+            }
+            else {
+                /*
+                 * If there are saved ID's the process might still be privileged
+                 * even though the above test succeeded. If issetugid() and
+                 * getresgid() aren't available, test this by trying to set
+                 * euid to 0.
+                 */
+                unsigned int oldeuid;
+
+                oldeuid = geteuid();
+
+                if (seteuid(0) != 0) {
+                    privsElevated = FALSE;
+                }
+                else {
+                    if (seteuid(oldeuid) != 0) {
+                        FatalError("Failed to drop privileges.  Exiting\n");
+                    }
+                    privsElevated = TRUE;
+                }
+            }
+#endif
+        }
+#endif
+        privsTested = TRUE;
+    }
+    return privsElevated;
+}
+
 /*
  * CheckUserParameters: check for long command line arguments and long
  * environment variables.  By default, these checks are only done when
  * the server's euid != ruid.  In 3.3.x, these checks were done in an
  * external wrapper utility.
  */
 
 /* Consider LD* variables insecure? */
 #ifndef REMOVE_ENV_LD
 #define REMOVE_ENV_LD 1

>From patchwork Fri Jan 27 13:37:36 2017
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Subject: [xserver,2/4] os: use PrivsElevated instead of a manual check
From: =?utf-8?q?Nicolai_H=C3=A4hnle?= <nhaehnle at gmail.com>
X-Patchwork-Id: 135711
Message-Id: <1485524258-6482-3-git-send-email-nhaehnle at gmail.com>
To: xorg-devel at lists.x.org
Cc: =?UTF-8?q?Nicolai=20H=C3=A4hnle?= <nicolai.haehnle at amd.com>
Date: Fri, 27 Jan 2017 14:37:36 +0100

From: Nicolai Hähnle <nicolai.haehnle at amd.com>

Signed-off-by: Nicolai Hähnle <nicolai.haehnle at amd.com>
---
 os/utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/os/utils.c b/os/utils.c
index 024989e..05733b0 100644
--- a/os/utils.c
+++ b/os/utils.c
@@ -1861,21 +1861,21 @@ enum BadCode {
 #endif
 
 void
 CheckUserParameters(int argc, char **argv, char **envp)
 {
     enum BadCode bad = NotBad;
     int i = 0, j;
     char *a, *e = NULL;
 
 #if CHECK_EUID
-    if (geteuid() == 0 && getuid() != geteuid())
+    if (PrivsElevated())
 #endif
     {
         /* Check each argv[] */
         for (i = 1; i < argc; i++) {
             if (strcmp(argv[i], "-fp") == 0) {
                 i++;            /* continue with next argument. skip the length check */
                 if (i >= argc)
                     break;
             }
             else {

>From patchwork Fri Jan 27 13:37:37 2017
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Subject: [xserver, 3/4] xfree86: replace all uses of xf86PrivsElevated with
 PrivsElevated
From: =?utf-8?q?Nicolai_H=C3=A4hnle?= <nhaehnle at gmail.com>
X-Patchwork-Id: 135712
Message-Id: <1485524258-6482-4-git-send-email-nhaehnle at gmail.com>
To: xorg-devel at lists.x.org
Cc: =?UTF-8?q?Nicolai=20H=C3=A4hnle?= <nicolai.haehnle at amd.com>
Date: Fri, 27 Jan 2017 14:37:37 +0100

From: Nicolai Hähnle <nicolai.haehnle at amd.com>

Signed-off-by: Nicolai Hähnle <nicolai.haehnle at amd.com>
---
 hw/xfree86/common/xf86Config.c |  2 +-
 hw/xfree86/common/xf86Init.c   | 12 +++---------
 hw/xfree86/common/xf86Priv.h   |  2 --
 3 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/hw/xfree86/common/xf86Config.c b/hw/xfree86/common/xf86Config.c
index f03acf3..5d2cf0a 100644
--- a/hw/xfree86/common/xf86Config.c
+++ b/hw/xfree86/common/xf86Config.c
@@ -2309,21 +2309,21 @@ xf86HandleConfigFile(Bool autoconfig)
 #endif
     Bool implicit_layout = FALSE;
     XF86ConfLayoutPtr layout;
 
     if (!autoconfig) {
         char *filename, *dirname, *sysdirname;
         const char *filesearch, *dirsearch;
         MessageType filefrom = X_DEFAULT;
         MessageType dirfrom = X_DEFAULT;
 
-        if (!xf86PrivsElevated()) {
+        if (!PrivsElevated()) {
             filesearch = ALL_CONFIGPATH;
             dirsearch = ALL_CONFIGDIRPATH;
         }
         else {
             filesearch = RESTRICTED_CONFIGPATH;
             dirsearch = RESTRICTED_CONFIGDIRPATH;
         }
 
         if (xf86ConfigFile)
             filefrom = X_CMDLINE;
diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c
index 758b926..deed91f 100644
--- a/hw/xfree86/common/xf86Init.c
+++ b/hw/xfree86/common/xf86Init.c
@@ -227,26 +227,20 @@ xf86PrintBanner(void)
 #if defined(BUILDERSTRING)
     xf86ErrorFVerb(0, "%s \n", BUILDERSTRING);
 #endif
     xf86ErrorFVerb(0, "Current version of pixman: %s\n",
                    pixman_version_string());
     xf86ErrorFVerb(0, "\tBefore reporting problems, check "
                    "" __VENDORDWEBSUPPORT__ "\n"
                    "\tto make sure that you have the latest version.\n");
 }
 
-Bool
-xf86PrivsElevated(void)
-{
-    return PrivsElevated();
-}
-
 static void
 TrapSignals(void)
 {
     if (xf86Info.notrapSignals) {
         OsSignal(SIGSEGV, SIG_DFL);
         OsSignal(SIGABRT, SIG_DFL);
         OsSignal(SIGILL, SIG_DFL);
 #ifdef SIGEMT
         OsSignal(SIGEMT, SIG_DFL);
 #endif
@@ -886,21 +880,21 @@ OsVendorInit(void)
     /* Set stderr to non-blocking. */
 #ifndef O_NONBLOCK
 #if defined(FNDELAY)
 #define O_NONBLOCK FNDELAY
 #elif defined(O_NDELAY)
 #define O_NONBLOCK O_NDELAY
 #endif
 
 #ifdef O_NONBLOCK
     if (!beenHere) {
-        if (xf86PrivsElevated()) {
+        if (PrivsElevated()) {
             int status;
 
             status = fcntl(fileno(stderr), F_GETFL, 0);
             if (status != -1) {
                 fcntl(fileno(stderr), F_SETFL, status | O_NONBLOCK);
             }
         }
     }
 #endif
 #endif
@@ -1041,21 +1035,21 @@ xf86PrintDefaultModulePath(void)
 
 static void
 xf86PrintDefaultLibraryPath(void)
 {
     ErrorF("%s\n", DEFAULT_LIBRARY_PATH);
 }
 
 static void
 xf86CheckPrivs(const char *option, const char *arg)
 {
-    if (xf86PrivsElevated() && !xf86PathIsSafe(arg)) {
+    if (PrivsElevated() && !xf86PathIsSafe(arg)) {
         FatalError("\nInvalid argument for %s - \"%s\"\n"
                     "\tWith elevated privileges %s must specify a relative path\n"
                     "\twithout any \"..\" elements.\n\n", option, arg, option);
     }
 }
 
 /*
  * ddxProcessArgument --
  *	Process device-dependent command line args. Returns 0 if argument is
  *      not device dependent, otherwise Count of number of elements of argv
@@ -1342,21 +1336,21 @@ ddxProcessArgument(int argc, char **argv, int i)
  *	Print out correct use of device dependent commandline options.
  *      Maybe the user now knows what really to do ...
  */
 
 void
 ddxUseMsg(void)
 {
     ErrorF("\n");
     ErrorF("\n");
     ErrorF("Device Dependent Usage\n");
-    if (!xf86PrivsElevated()) {
+    if (!PrivsElevated()) {
         ErrorF("-modulepath paths      specify the module search path\n");
         ErrorF("-logfile file          specify a log file name\n");
         ErrorF("-configure             probe for devices and write an "
                XCONFIGFILE "\n");
         ErrorF
             ("-showopts              print available options for all installed drivers\n");
     }
     ErrorF
         ("-config file           specify a configuration file, relative to the\n");
     ErrorF("                       " XCONFIGFILE
diff --git a/hw/xfree86/common/xf86Priv.h b/hw/xfree86/common/xf86Priv.h
index c1f8a18..d78e8f6 100644
--- a/hw/xfree86/common/xf86Priv.h
+++ b/hw/xfree86/common/xf86Priv.h
@@ -154,16 +154,14 @@ xf86CloseLog(enum ExitCode error);
 
 /* xf86Init.c */
 extern _X_EXPORT Bool
 xf86LoadModules(const char **list, void **optlist);
 extern _X_EXPORT int
 xf86SetVerbosity(int verb);
 extern _X_EXPORT int
 xf86SetLogVerbosity(int verb);
 extern _X_EXPORT Bool
 xf86CallDriverProbe(struct _DriverRec *drv, Bool detect_only);
-extern _X_EXPORT Bool
-xf86PrivsElevated(void);
 
 #endif                          /* _NO_XF86_PROTOTYPES */
 
 #endif                          /* _XF86PRIV_H */

>From patchwork Fri Jan 27 13:37:38 2017
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Subject: [xserver,4/4] glx: honor LIBGL_DRIVERS_PATH when loading DRI drivers
From: =?utf-8?q?Nicolai_H=C3=A4hnle?= <nhaehnle at gmail.com>
X-Patchwork-Id: 135713
Message-Id: <1485524258-6482-5-git-send-email-nhaehnle at gmail.com>
To: xorg-devel at lists.x.org
Cc: =?UTF-8?q?Nicolai=20H=C3=A4hnle?= <nicolai.haehnle at amd.com>
Date: Fri, 27 Jan 2017 14:37:38 +0100

From: Nicolai Hähnle <nicolai.haehnle at amd.com>

Allow switching to another driver build without a full installation.

Glamor already takes LIBGL_DRIVERS_PATH into account, so this change
makes sure that the same driver is used in both parts of the server.

Signed-off-by: Nicolai Hähnle <nicolai.haehnle at amd.com>
---
 glx/glxdricommon.c | 38 ++++++++++++++++++++++++++++++++++----
 1 file changed, 34 insertions(+), 4 deletions(-)

diff --git a/glx/glxdricommon.c b/glx/glxdricommon.c
index f6c6fcd..a370845 100644
--- a/glx/glxdricommon.c
+++ b/glx/glxdricommon.c
@@ -241,28 +241,58 @@ static const char dri_driver_path[] = DRI_DRIVER_PATH;
 void *
 glxProbeDriver(const char *driverName,
                void **coreExt, const char *coreName, int coreVersion,
                void **renderExt, const char *renderName, int renderVersion)
 {
     int i;
     void *driver;
     char filename[PATH_MAX];
     char *get_extensions_name;
     const __DRIextension **extensions = NULL;
+    const char *path = NULL;
+
+    /* Search in LIBGL_DRIVERS_PATH if we're not setuid. */
+    if (!PrivsElevated())
+        path = getenv("LIBGL_DRIVERS_PATH");
+
+    if (!path)
+        path = dri_driver_path;
+
+    do {
+        const char *next;
+        int path_len;
+
+        next = strchr(path, ':');
+        if (next) {
+            path_len = next - path;
+            next++;
+        } else {
+            path_len = strlen(path);
+            next = NULL;
+        }
 
-    snprintf(filename, sizeof filename, "%s/%s_dri.so",
-             dri_driver_path, driverName);
+        snprintf(filename, sizeof filename, "%.*s/%s_dri.so", path_len, path,
+                 driverName);
+
+        driver = dlopen(filename, RTLD_LAZY | RTLD_LOCAL);
+        if (driver != NULL)
+            break;
 
-    driver = dlopen(filename, RTLD_LAZY | RTLD_LOCAL);
-    if (driver == NULL) {
         LogMessage(X_ERROR, "AIGLX error: dlopen of %s failed (%s)\n",
                    filename, dlerror());
+
+        path = next;
+    } while (path);
+
+    if (driver == NULL) {
+        LogMessage(X_ERROR, "AIGLX error: unable to load driver %s\n",
+                  driverName);
         goto cleanup_failure;
     }
 
     if (asprintf(&get_extensions_name, "%s_%s",
                  __DRI_DRIVER_GET_EXTENSIONS, driverName) != -1) {
         const __DRIextension **(*get_extensions)(void);
 
         get_extensions = dlsym(driver, get_extensions_name);
         if (get_extensions)
             extensions = get_extensions();


More information about the xorg-devel mailing list