Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xrdp compiling on Solaris, two changes required #2908

Closed
abrossard opened this issue Jan 9, 2024 · 3 comments · Fixed by #2909
Closed

xrdp compiling on Solaris, two changes required #2908

abrossard opened this issue Jan 9, 2024 · 3 comments · Fixed by #2909
Labels

Comments

@abrossard
Copy link

abrossard commented Jan 9, 2024

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 <limits.h> 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

@abrossard abrossard added the bug label Jan 9, 2024
@matt335672
Copy link
Member

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
Copy link
Member

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
Copy link
Member

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants