leaks file descriptors

[mirabilos]

I just found these in my X session log:

icewm-session: Warning: File still open: fd=3, target='/var/log/xrdp-sesman.log' (missing FD_CLOEXEC?) icewm-session: Warning: File still open: fd=5, target='socket:[527965]' (missing FD_CLOEXEC?) icewm-session: Warning: File still open: fd=6, target='socket:[527967]' (missing FD_CLOEXEC?)

Apparently, xrdp-sesman leaks fds to the user session. This should be fixed, FD_CLOEXEC is a good idea. You can apply it to both files (fd, not FILE*) and sockets after opening like this (taken from my own mksh project):

fcntl(fd, F_SETFD, FD_CLOEXEC);

[jsorg71]

We can add FD_CLOEXEC but is that available on all platforms that run sesman? If we add that,it's ok but I would also like to find and close it. The log is for sure fixable. I'll check on the 2 sockets.

[mirabilos]

jsorg71 dixit:

We can add FD_CLOEXEC but is that available on all platforms that run sesman?

It’s extremely ubiquitous (I’ve yet to see one that lacks it). You could safeguard it if needed:

#ifdef FD_CLOEXEC
fcntl(fd, F_SETFD, FD_CLOEXEC);
#endif

But basically, if you have it’ll be there.

The log is for sure fixable. I'll check on the 2 sockets.

Maybe the sockets need WsaSomeFunction() on Win32, but other than that, I think it’d work.

bye,

//mirabilos

exceptions: a truly awful implementation of quite a nice idea. just about the worst way you could do something like that, afaic. it's like anti-design. that too… may I quote you on that? sure, tho i doubt anyone will listen ;)

[jsorg71]

I think this is fixed in devel now with 7889ee638e2d9549d6cb98a82365405c66f74fec

[moobyfr]

Could this fix be able to get rid of some zombies process (xrdp-chansrv, attached from xrdp-sesman)?

[mirabilos]

Blindauer Emmanuel dixit:

Could this fix be able to get rid of some zombies process (xrdp-chansrv, attached from xrdp-sesman)?

Hm, we’ve had these in our testing only while (IIRC) gnome-keyring was active, not with the shipped PAM config. Nik, can you confirm?

[mirabilos]

Hi,

Could this fix be able to get rid of some zombies process (xrdp-chansrv, attached from xrdp-sesman)?

Hm, we’ve had these in our testing only while (IIRC) gnome-keyring was active, not with the shipped PAM config. Nik, can you confirm?

While I can confirm we identified gnome-keyring as a culprit, I am sure that it was not a bug specific to gnome-keyring, but to something gnome-keyring triggered.

So I assume that we should test a patch that addresses the root cause of the issue.

-nik

PGP-Fingerprint: 3C9D 54A4 7575 C026 FB17 FD26 B79A 3C16 A0C4 F296

Dominik George · Mobil: +49-151-61623918

Teckids e.V. · FrOSCon e.V. · OpenRheinRuhr e.V. Fellowship of the FSFE · Piratenpartei Deutschland Opencaching Deutschland e.V. · Debian Contributor

LPIC-3 Linux Enterprise Professional (Security)

[mirabilos]

Dominik George dixit:

So I assume that we should test a patch that addresses the root cause of the issue.

No, the patch was to fix a file descriptor leak to the window manager.

bye, //mirabilos

[metalefty]

It seems to be fixed. Please reopen or open new issue if it's not fixed.