xserver: Branch 'master' - 4 commits

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Thu Jan 26 17:53:12 UTC 2023


 include/meson.build |    1 
 os/access.c         |   46 +++++++++++++++------
 os/client.c         |  110 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 os/connection.c     |    4 -
 4 files changed, 144 insertions(+), 17 deletions(-)

New commits:
commit 0ea9b595891f2f31915538192961f3404d9ca699
Author: Jeremy Huddleston Sequoia <jeremyhu at apple.com>
Date:   Wed Jan 18 10:38:41 2023 -0800

    darwin: Implement DetermineClientCmd for macOS
    
    Withoug a proper implementation of DetermineClientCmd, clients that
    connect via an ssh tunnel are miscategorized as local.  This results
    in failures when we try to use SCM_RIGHTS (eg: in MIT-SHM).
    
    Fixes: https://github.com/XQuartz/XQuartz/issues/314
    Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu at apple.com>

diff --git a/os/client.c b/os/client.c
index 89a92d5b5..922172cc5 100644
--- a/os/client.c
+++ b/os/client.c
@@ -73,6 +73,12 @@
 #include <limits.h>
 #endif
 
+#ifdef __APPLE__
+#include <dispatch/dispatch.h>
+#include <errno.h>
+#include <sys/sysctl.h>
+#endif
+
 /**
  * Try to determine a PID for a client from its connection
  * information. This should be called only once when new client has
@@ -130,9 +136,11 @@ DetermineClientPid(struct _Client * client)
 void
 DetermineClientCmd(pid_t pid, const char **cmdname, const char **cmdargs)
 {
+#if !defined(__APPLE__)
     char path[PATH_MAX + 1];
     int totsize = 0;
     int fd = 0;
+#endif
 
     if (cmdname)
         *cmdname = NULL;
@@ -142,7 +150,107 @@ DetermineClientCmd(pid_t pid, const char **cmdname, const char **cmdargs)
     if (pid == -1)
         return;
 
-#if defined(__OpenBSD__)
+#if defined (__APPLE__)
+    {
+        static dispatch_once_t once;
+        static int argmax;
+        dispatch_once(&once, ^{
+            int mib[2];
+            size_t len;
+
+            mib[0] = CTL_KERN;
+            mib[1] = KERN_ARGMAX;
+
+            len = sizeof(argmax);
+            if (sysctl(mib, 2, &argmax, &len, NULL, 0) == -1) {
+                ErrorF("Unable to dynamically determine kern.argmax, using ARG_MAX (%d)\n", ARG_MAX);
+                argmax = ARG_MAX;
+            }
+        });
+
+        int mib[3];
+        size_t len = argmax;
+        int32_t argc = -1;
+
+        char * const procargs = malloc(len);
+        if (!procargs) {
+            ErrorF("Failed to allocate memory (%lu bytes) for KERN_PROCARGS2 result for pid %d: %s\n", len, pid, strerror(errno));
+            return;
+        }
+
+        mib[0] = CTL_KERN;
+        mib[1] = KERN_PROCARGS2;
+        mib[2] = pid;
+
+        if (sysctl(mib, 3, procargs, &len, NULL, 0) == -1) {
+            ErrorF("Failed to determine KERN_PROCARGS2 for pid %d: %s\n", pid, strerror(errno));
+            free(procargs);
+            return;
+        }
+
+        if (len < sizeof(argc) || len > argmax) {
+            ErrorF("Erroneous length returned when querying KERN_PROCARGS2 for pid %d: %zu\n", pid, len);
+            free(procargs);
+            return;
+        }
+
+        /* Ensure we have a failsafe NUL termination just in case the last entry
+         * was not actually NUL terminated.
+         */
+        procargs[len-1] = '\0';
+
+        /* Setup our iterator */
+        char *is = procargs;
+
+        /* The first element in the buffer is argc as a 32bit int. When using
+         * the older KERN_PROCARGS, this is omitted, and one needs to guess
+         * (usually by checking for an `=` character) when we start seeing
+         * envvars instead of arguments.
+         */
+        argc = *(int32_t *)is;
+        is += sizeof(argc);
+
+        /* The very next string is the executable path.  Skip over it since
+         * this function wants to return argv[0] and argv[1...n].
+         */
+        is += strlen(is) + 1;
+
+        /* Skip over extra NUL characters to get to the start of argv[0] */
+        for (; (is < &procargs[len]) && !(*is); is++);
+
+        if (! (is < &procargs[len])) {
+            ErrorF("Arguments were not returned when querying KERN_PROCARGS2 for pid %d: %zu\n", pid, len);
+            free(procargs);
+            return;
+        }
+
+        if (cmdname) {
+            *cmdname = strdup(is);
+        }
+
+        /* Jump over argv[0] and point to argv[1] */
+        is += strlen(is) + 1;
+
+        if (cmdargs && is < &procargs[len]) {
+            char *args = is;
+
+            /* Remove the NUL terminators except the last one */
+            for (int i = 1; i < argc - 1; i++) {
+                /* Advance to the NUL terminator */
+                is += strlen(is);
+
+                /* Change the NUL to a space, ensuring we don't accidentally remove the terminal NUL */
+                if (is < &procargs[len-1]) {
+                    *is = ' ';
+                }
+            }
+
+            *cmdargs = strdup(args);
+        }
+
+        free(procargs);
+    }
+#elif defined(__OpenBSD__)
     /* on OpenBSD use kvm_getargv() */
     {
         kvm_t *kd;
commit 8a4ab2287398773a4868c220662d93bf84ec6241
Author: Jeremy Huddleston Sequoia <jeremyhu at apple.com>
Date:   Wed Jan 18 12:19:05 2023 -0800

    os: Use LOCAL_PEERPID from sys/un.h if it is available to detemine the pid when falling back on getpeereids()
    
    This provides a way to determine the pid of a peer connection on
    systems like darwin that do not support getpeerucred() nor
    SO_PEERCRED.
    
    Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu at apple.com>

diff --git a/include/meson.build b/include/meson.build
index 855fbba5d..e953dfa65 100644
--- a/include/meson.build
+++ b/include/meson.build
@@ -133,6 +133,7 @@ conf_data.set('HAVE_FNMATCH_H', cc.has_header('fnmatch.h') ? '1' : false)
 conf_data.set('HAVE_LINUX_AGPGART_H', cc.has_header('linux/agpgart.h') ? '1' : false)
 conf_data.set('HAVE_STRINGS_H', cc.has_header('strings.h') ? '1' : false)
 conf_data.set('HAVE_SYS_AGPGART_H', cc.has_header('sys/agpgart.h') ? '1' : false)
+conf_data.set('HAVE_SYS_UN_H', cc.has_header('sys/un.h') ? '1' : false)
 conf_data.set('HAVE_SYS_UTSNAME_H', cc.has_header('sys/utsname.h') ? '1' : false)
 conf_data.set('HAVE_SYS_SYSMACROS_H', cc.has_header('sys/sysmacros.h') ? '1' : false)
 
diff --git a/os/access.c b/os/access.c
index debf4c741..61ee8e30f 100644
--- a/os/access.c
+++ b/os/access.c
@@ -116,6 +116,10 @@ SOFTWARE.
 #endif
 #endif
 
+#ifdef HAVE_SYS_UN_H
+#include <sys/un.h>
+#endif
+
 #if defined(SVR4) ||  (defined(SYSV) && defined(__i386__)) || defined(__GNU__)
 #include <sys/utsname.h>
 #endif
@@ -1176,6 +1180,10 @@ GetLocalClientCreds(ClientPtr client, LocalClientCredRec ** lccp)
 #elif defined(HAVE_GETPEEREID)
     uid_t uid;
     gid_t gid;
+#if defined(LOCAL_PEERPID)
+    pid_t pid;
+    socklen_t so_len = sizeof(pid);
+#endif
 #endif
 
     if (client == NULL)
@@ -1253,6 +1261,16 @@ GetLocalClientCreds(ClientPtr client, LocalClientCredRec ** lccp)
     lcc->euid = uid;
     lcc->egid = gid;
     lcc->fieldsSet = LCC_UID_SET | LCC_GID_SET;
+
+#if defined(LOCAL_PEERPID)
+    if (getsockopt(fd, SOL_LOCAL, LOCAL_PEERPID, &pid, &so_len) != 0) {
+        ErrorF("getsockopt failed to determine pid of socket %d: %s\n", fd, strerror(errno));
+    } else {
+        lcc->pid = pid;
+        lcc->fieldsSet |= LCC_PID_SET;
+    }
+#endif
+
     return 0;
 #endif
 #else
commit 165d5c1260edcb998c5cf31d3969723c7452aa7f
Author: Jeremy Huddleston Sequoia <jeremyhu at apple.com>
Date:   Wed Jan 18 12:02:54 2023 -0800

    os: Update GetLocalClientCreds to prefer getpeerucred() or SO_PEERCRED over getpeereid()
    
    GetLocalClientCreds() was preferring getpeereid() above other implementations.
    
    getpeereid(), however, only returns the effective uid and gid of the peer,
    leaving the pid unset.  When this happens, we are unable to use the pid to
    determine the peer's command line arguments and incorrectly treat ssh-tunneled
    traffic as local.
    
    To address this, we now prioritize getpeerucred() or SO_PEERCRED as those two
    implementations will return the pid in addition to uid and gid.
    
    Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu at apple.com>

diff --git a/os/access.c b/os/access.c
index b1703048d..debf4c741 100644
--- a/os/access.c
+++ b/os/access.c
@@ -1167,15 +1167,15 @@ GetLocalClientCreds(ClientPtr client, LocalClientCredRec ** lccp)
     XtransConnInfo ci;
     LocalClientCredRec *lcc;
 
-#ifdef HAVE_GETPEEREID
-    uid_t uid;
-    gid_t gid;
-#elif defined(HAVE_GETPEERUCRED)
+#if defined(HAVE_GETPEERUCRED)
     ucred_t *peercred = NULL;
     const gid_t *gids;
 #elif defined(SO_PEERCRED)
     struct ucred peercred;
     socklen_t so_len = sizeof(peercred);
+#elif defined(HAVE_GETPEEREID)
+    uid_t uid;
+    gid_t gid;
 #endif
 
     if (client == NULL)
@@ -1197,16 +1197,7 @@ GetLocalClientCreds(ClientPtr client, LocalClientCredRec ** lccp)
     lcc = *lccp;
 
     fd = _XSERVTransGetConnectionNumber(ci);
-#ifdef HAVE_GETPEEREID
-    if (getpeereid(fd, &uid, &gid) == -1) {
-        FreeLocalClientCreds(lcc);
-        return -1;
-    }
-    lcc->euid = uid;
-    lcc->egid = gid;
-    lcc->fieldsSet = LCC_UID_SET | LCC_GID_SET;
-    return 0;
-#elif defined(HAVE_GETPEERUCRED)
+#if defined(HAVE_GETPEERUCRED)
     if (getpeerucred(fd, &peercred) < 0) {
         FreeLocalClientCreds(lcc);
         return -1;
@@ -1254,6 +1245,15 @@ GetLocalClientCreds(ClientPtr client, LocalClientCredRec ** lccp)
     lcc->pid = peercred.pid;
     lcc->fieldsSet = LCC_UID_SET | LCC_GID_SET | LCC_PID_SET;
     return 0;
+#elif defined(HAVE_GETPEEREID)
+    if (getpeereid(fd, &uid, &gid) == -1) {
+        FreeLocalClientCreds(lcc);
+        return -1;
+    }
+    lcc->euid = uid;
+    lcc->egid = gid;
+    lcc->fieldsSet = LCC_UID_SET | LCC_GID_SET;
+    return 0;
 #endif
 #else
     /* No system call available to get the credentials of the peer */
commit 2577291f010e07173d0fc8b310ac355928f8ed7d
Author: Jeremy Huddleston Sequoia <jeremyhu at apple.com>
Date:   Wed Jan 18 10:44:27 2023 -0800

    os: Update AllocNewConnection() debug logging to include whether or not the client is local
    
    Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu at apple.com>

diff --git a/os/connection.c b/os/connection.c
index 1d92b6013..a3c8f2a99 100644
--- a/os/connection.c
+++ b/os/connection.c
@@ -637,8 +637,8 @@ AllocNewConnection(XtransConnInfo trans_conn, int fd, CARD32 conn_time)
     set_poll_client(client);
 
 #ifdef DEBUG
-    ErrorF("AllocNewConnection: client index = %d, socket fd = %d\n",
-           client->index, fd);
+    ErrorF("AllocNewConnection: client index = %d, socket fd = %d, local = %d\n",
+           client->index, fd, client->local);
 #endif
 #ifdef XSERVER_DTRACE
     XSERVER_CLIENT_CONNECT(client->index, fd);


More information about the xorg-commit mailing list