neutrinolabs / xrdp

xrdp: an open source RDP server
http://www.xrdp.org/
Apache License 2.0
5.67k stars 1.73k forks source link

xrdp compiling on Solaris, two changes required #2908

Closed abrossard closed 9 months ago

abrossard commented 9 months ago

xrdp version

0.9.80

Detailed xrdp version, build options

xrdp 0.9.80
  A Remote Desktop Protocol Server.
  Copyright (C) 2004-2020 Jay Sorg, Neutrino Labs, and all contributors.
  See https://github.com/neutrinolabs/xrdp for more information.

  Configure options:

  Compiled with OpenSSL 1.0.2zh  30 May 2023

Operating system & version

Solaris 11.4.62

Installation method

git clone & make install

Which backend do you use?

tbd

What desktop environment do you use?

gnove

Environment xrdp running on

Solaris zones

What's your client?

tbd

Area(s) with issue?

Crashes such as segfault, Compile error

Steps to reproduce

The first issue is the requirements for in os_calls.c in order to compile. In the same file, the second issue is the method used to clear the environment is "harsh" (environ=0) and cause a core on Solaris, it is better to use clearenv which would work everywhere.

diff --git a/common/os_calls.c b/common/os_calls.c
index 765ad472..624e0621 100644
--- a/common/os_calls.c
+++ b/common/os_calls.c
@@ -35,6 +35,7 @@
 /* fix for solaris 10 with gcc 3.3.2 problem */
 #if defined(sun) || defined(__sun)
 #define ctid_t id_t
+#include <limits.h>
 #endif
 #include <unistd.h>
 #include <errno.h>
@@ -3450,10 +3451,14 @@ g_clearenv(void)
 #else
 #if defined(BSD)
     environ[0] = 0;
+#else
+#if defined(sun) || defined(__sun)
+    clearenv();
 #else
     environ = 0;
 #endif
 #endif
+#endif
 }

 /*****************************************************************************/

✔️ Expected Behavior

attempt to start the Xorg server

❌ Actual Behavior

core because of "environ=0" in g_clearenv() [2024-01-09T09:27:46.976+0100] [ERROR] waitforx failed with unexpected signal SIGSEGV

Anything else?

No response

matt335672 commented 9 months ago

Thanks @abrossard

We had some Solaris stuff going in #2811, but that's gone quiet. I'll chase it up.

I remember coming across this myself as a Solaris system admin many years ago, and coming up with a hacky workaround. Your fix looks better.

Can you tell me what compiler you're using and what the error is when limits.h is not included? I'll put together a PR for you to try for us.

matt335672 commented 9 months ago

Found the limits.h thing.

On Linux, including <sys/param.h> is pulling in limits.h which is needed for USHRT_MAX. I think this warrants a separate include regardless.

matt335672 commented 9 months ago

@abrossard - can you test out the patch from #2909 and let us know if that fixes your immediate concerns? If it's good, I'll merge it.

Patch is available directly as text here:-

https://github.com/neutrinolabs/xrdp/commit/06af6123e048aa8cf0433123e77774d0d894590a.patch