[dev.icinga.com #8900] File descriptors are leaked to child processes which makes SELinux unhappy

[icinga-migration]

This issue has been migrated from Redmine: https://dev.icinga.com/issues/8900

Created by dgoetz on 2015-03-30 08:13:21 +00:00

Assignee: gbeutner
Status: Resolved (closed on 2016-10-04 12:10:04 +00:00)
Target Version: 2.6.0
Last Update: 2017-01-05 23:00:49 +00:00 (in Redmine)

Icinga Version: 2.3.2
Backport?: Not yet backported
Include in Changelog: 1

---

If feature perfdata is activated it leaks: /var/spool/icinga2/tmp/service-perfdata and /var/spool/icinga2/tmp/host-perfdata
If feature api is enabled it leaks: /var/lib/icinga2/api/log/current

Attachments

• leaking-file-descriptors-8900.patch jyoung15 - 2015-11-16 04:33:57 +00:00

Changesets

2016-10-04 12:08:48 +00:00 by gbeutner a7b0cb5

Ensure we don't leak file descriptors to child processes

fixes #8900

2016-10-05 13:05:28 +00:00 by jflach 541c67d

Don't use InitializeSpawnHelper on Windows

refs #8900

2016-10-05 13:10:43 +00:00 by jflach 069de6c

Don't use InitializeSpawnHelper on Windows

refs #8900

---

Relations:

• relates #8900
• relates #8900
• relates #8900
• relates #8900

[icinga-migration]

Updated by dgoetz on 2015-03-30 08:15:11 +00:00

• Relates set to 8332

[icinga-migration]

Updated by mfriedrich on 2015-04-17 16:00:35 +00:00

• Relates set to 8545

[icinga-migration]

Updated by mfrosch on 2015-08-26 13:23:31 +00:00

• Target Version set to Backlog

We should consider this when we have time :) (Maybe me)

[icinga-migration]

Updated by mfriedrich on 2015-09-05 14:44:08 +00:00

• Category set to libmethods
• Priority changed from Normal to High
• Target Version deleted Backlog

[icinga-migration]

Updated by jyoung15 on 2015-11-16 04:53:32 +00:00

• File added leaking-file-descriptors-8900.patch

Attached is a proposed patch to close all open file descriptors (except STDIN/STDOUT/STDERR) after forking a new child process. Uses closefrom(2) on FreeBSD and emulates the same for Linux/OSX.

Tested on FreeBSD and Linux. Prior to applying patch 'sudo lsof /var/lib/icinga2/api/log/current' indicated plugin processes had this file open. After applying patch this is no longer seen.

Needs further peer testing to confirm no bugs are introduced.

Portions of code borrowed from cocaine project

[icinga-migration]

Updated by gbeutner on 2016-02-10 07:01:35 +00:00

Things to consider before merging this patch: There are a number of things we cannot do after vfork(), e.g. allocating memory (which is probably something boost::filesystem does).

[icinga-migration]

Updated by mfriedrich on 2016-03-04 16:54:01 +00:00

• Target Version set to Backlog

[icinga-migration]

Updated by tgelf on 2016-03-31 11:46:26 +00:00

**

This issue is a severe show-stopper for SELinux and therefore for Icinga2 as a whole in security-sensitive environments.

Cheers,
Thomas

[icinga-migration]

Updated by mfriedrich on 2016-04-04 07:42:13 +00:00

• Priority changed from High to Normal
• Target Version changed from Backlog to 2.5.0

[icinga-migration]

Updated by Gaston on 2016-04-15 07:20:20 +00:00

maybe related:

Blog post from Dan Walsh about SELinux and the FD leaking problem: http://danwalsh.livejournal.com/53603.html

proposes to use O_CLOEXEC to auto-close on FD on fork/exec

[icinga-migration]

Updated by gbeutner on 2016-06-14 06:28:18 +00:00

Correct me if I'm wrong but there doesn't seem to be a portable way to get the fd for MySQL connections - and iirc MySQL doesn't use O_CLOEXEC for its client connections.

[icinga-migration]

Updated by mfriedrich on 2016-06-16 08:44:58 +00:00

• Target Version changed from 2.5.0 to 2.6.0

[icinga-migration]

Updated by gbeutner on 2016-09-29 07:21:53 +00:00

• Category changed from libmethods to libbase
• Status changed from New to Assigned
• Assigned to set to gbeutner

[icinga-migration]

Updated by gbeutner on 2016-10-04 12:10:04 +00:00

• Status changed from Assigned to Resolved
• Done % changed from 0 to 100

Applied in changeset a7b0cb5.

[icinga-migration]

Updated by jyoung15 on 2016-10-18 05:26:09 +00:00

a7b0cb5 may have broken some plugins that rely on alarm(). The sigfillset/sigprocmask in ProcessHandler causes SIGALRM to be masked. I'm not sure about the impact on standard plugins, but it has affected a few of my custom plugins. Not sure if anyone else is affected by this.

static void ProcessHandler(void)
{
       sigset_t mask;
       sigfillset(&mask);
       sigprocmask(SIG_SETMASK, &mask, NULL);

[icinga-migration]

Updated by gbeutner on 2016-10-18 06:11:20 +00:00

Hm, interesting, can you submit a separate ticket for that please and I'll have a look so we can get this fixed before the release. :)

[icinga-migration]

Updated by jyoung15 on 2016-10-18 14:19:19 +00:00

Thanks. I submitted #12940

[icinga-migration]

Updated by mfriedrich on 2016-11-21 15:12:01 +00:00

• Relates set to 13269

[icinga-migration]

Updated by gbeutner on 2016-12-12 12:23:28 +00:00

• Subject changed from Icinga2 is leaking file descriptor to the plugins depending on activated features to File descriptors are leaked to child processes which makes SELinux unhappy

[icinga-migration]

Updated by mfriedrich on 2016-12-15 10:52:15 +00:00

• Relates set to 13567

[icinga-migration]

Updated by lucasmopdx on 2017-01-05 23:00:49 +00:00

This appears to cause a segfault for spawning any child processes when their total environment, expressed in JSON, exceeds 4096 bytes in size. See #13655 for further analysis.