From 0d484bea65ca03056fb138c4ecfcc845ee0088c6 Mon Sep 17 00:00:00 2001 From: DRC Date: Tue, 17 Sep 2024 13:33:04 -0400 Subject: [PATCH] Server: Fix inetd service issues ... introduced when we migrated to the xorg-server 1.19.x code base in TurboVNC 2.2 (a661ed1f3a1c887f1de41b17a1749dc7568f5c8c). - Use dup() to obtain a free file descriptor for the inetd socket rather than statically assigning FD 3. Otherwise, if Xvnc calls a function (such as getpwuid()) that invokes libnss_sss prior to calling ddxProcessArgument(), then libnss_sss will obtain the first free file descriptor (3) for its own purposes, ddxProcessArgument() will clobber that file descriptor, and hilarity will ensue. TurboVNC doesn't currently make an early call to a function that invokes libnss_sss, but referring to https://github.com/tigervnc/tigervnc/issues/1709, TigerVNC does. It is possible that we will do so in the future. Based on: https://github.com/TigerVNC/tigervnc/commit/7a9773a303458920a1dfc49e1406e254935ad42d - Explicitly redirect stderr (FD 2) to /dev/null rather than simply closing it. Otherwise, OsInit() (more specifically ospoll_create()) will obtain the first free file descriptor (2) to use for polling, then it will check whether FD 2 (which it assumes is still stderr) is writable. Since polling file descriptors aren't writable, OsInit() will redirect FD 2 to /dev/null, thus breaking the polling subsystem. (Symptomatically, that caused the TurboVNC Server, when used with the -inetd option, to do nothing when a VNC viewer connected.) Based on: https://github.com/TigerVNC/tigervnc/commit/712cf8673d6e57442f41636e44020f5e1839c7f8 - Move the inetd RFB connection code from ProcessInputEvents() to the end of InitInput(), since ProcessInputEvents() isn't usually called until/unless a window manager is started. (Symptomatically, that also caused the TurboVNC Server, when used with the -inetd option, to do nothing when a VNC viewer connected.) Probably fixes #303 --- ChangeLog.md | 3 ++ unix/Xvnc/programs/Xserver/hw/vnc/init.c | 43 +++++++++++++++--------- 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/ChangeLog.md b/ChangeLog.md index bac4b4901..dd44cfb5c 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -83,6 +83,9 @@ from being used as VNC hostnames. redirect all TurboVNC Server and X.org errors, warnings, and messages to the system log. +16. Fixed a regression introduced by 2.2 beta1[7] that prevented the TurboVNC +Server from being used as an inetd service. + 3.0.3 ===== diff --git a/unix/Xvnc/programs/Xserver/hw/vnc/init.c b/unix/Xvnc/programs/Xserver/hw/vnc/init.c index d61e02a36..65d73812d 100644 --- a/unix/Xvnc/programs/Xserver/hw/vnc/init.c +++ b/unix/Xvnc/programs/Xserver/hw/vnc/init.c @@ -263,7 +263,8 @@ int ddxProcessArgument(int argc, char *argv[], int i) } if (strcasecmp(argv[i], "-inetd") == 0) { /* -inetd */ - int n; + int n, nullFD; + for (n = 1; n < 100; n++) { if (CheckDisplayNumber(n)) break; @@ -275,14 +276,26 @@ int ddxProcessArgument(int argc, char *argv[], int i) snprintf(inetdDisplayNumStr, 10, "%d", n); display = inetdDisplayNumStr; - /* fds 0, 1 and 2 (stdin, out and err) are all the same socket to the - RFB client. OsInit() closes stdout and stdin, and we don't want - stderr to go to the RFB client, so make the client socket 3 and - close stderr. OsInit() will redirect stderr logging to an - appropriate log file or /dev/null if that doesn't work. */ - dup2(0, 3); - inetdSock = 3; - close(2); + /* FDs 0, 1, and 2 (stdin, stdout, and stderr) are all redirected to the + same socket, which is connected to the RFB client. OsInit() closes + stdin and stdout, so we obtain a new file descriptor for the socket by + duplicating stdin. */ + if ((inetdSock = dup(0)) == -1) + FatalError("-inetd: couldn't allocate a new file descriptor: %s", + strerror(errno)); + + /* We don't want stderr to be redirected to the RFB client, since stderr is + used for logging. (NOTE: The -syslog option redirects most of the + logging to the system log, but it isn't foolproof.) We explicitly + redirect stderr (FD 2) to /dev/null rather than simply closing it. + Otherwise, OsInit() (more specifically ospoll_create()) will obtain the + first free file descriptor (2) to use for polling, then it will check + whether FD 2 (which it assumes is still stderr) is writable. Since + polling file descriptors aren't writable, OsInit() will redirect FD 2 to + /dev/null, thus breaking the polling subsystem. */ + nullFD = open("/dev/null", O_WRONLY); + dup2(nullFD, 2); + close(nullFD); return 1; } @@ -1125,6 +1138,7 @@ rfbDevInfo virtualTabletPad = void InitInput(int argc, char *argv[]) { DeviceIntPtr p, k; + static Bool inetdInitDone = FALSE; if (AllocDevicePair(serverClient, "TurboVNC", &p, &k, rfbMouseProc, rfbKeybdProc, FALSE) != Success) @@ -1150,6 +1164,11 @@ void InitInput(int argc, char *argv[]) if (!AddExtInputDevice(&virtualTabletPad)) FatalError("Could not create TurboVNC virtual tablet pad device"); } + + if (!inetdInitDone && inetdSock != -1) { + rfbNewClientConnection(inetdSock); + inetdInitDone = TRUE; + } } @@ -1409,12 +1428,6 @@ Bool LegalModifier(unsigned int key, DeviceIntPtr pDev) void ProcessInputEvents(void) { - static Bool inetdInitDone = FALSE; - - if (!inetdInitDone && inetdSock != -1) { - rfbNewClientConnection(inetdSock); - inetdInitDone = TRUE; - } mieqProcessInputEvents(); }