Icinga / icinga2

The core of our monitoring platform with a powerful configuration language and REST API.
https://icinga.com/docs/icinga2/latest
GNU General Public License v2.0
2.03k stars 578 forks source link

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

Closed icinga-migration closed 8 years ago

icinga-migration commented 9 years ago

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

Changesets

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

Ensure we don't leak file descriptors to child processes

fixes #8900

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

Don't use InitializeSpawnHelper on Windows

refs #8900

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

Don't use InitializeSpawnHelper on Windows

refs #8900

Relations:

icinga-migration commented 9 years ago

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

icinga-migration commented 9 years ago

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

icinga-migration commented 9 years ago

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

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

icinga-migration commented 9 years ago

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

icinga-migration commented 9 years ago

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

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 commented 8 years ago

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 commented 8 years ago

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

icinga-migration commented 8 years ago

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 commented 8 years ago

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

icinga-migration commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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

icinga-migration commented 8 years ago

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

icinga-migration commented 8 years ago

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

Applied in changeset a7b0cb5f7e790b1e9904a933dbdeedfaf5b78357.

icinga-migration commented 8 years ago

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

a7b0cb5f 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 commented 8 years ago

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 commented 8 years ago

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

Thanks. I submitted #12940

icinga-migration commented 8 years ago

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

icinga-migration commented 7 years ago

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

icinga-migration commented 7 years ago

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

icinga-migration commented 7 years ago

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.