From 15a900327aba7dc4dc886affe1ae22d3b759b193 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 8 Feb 2016 22:30:58 +0100 Subject: core: set RLIMIT_CORE to unlimited by default The kernel sets RLIMIT_CORE to 0 by default. Let's bump this to unlimited by default (for systemd itself and all processes we fork off), so that the coredump hooks have an effect if they honour it. Bumping RLIMIT_CORE of course would have the effect that "core" files will end up on the system at various places, if no coredump hook is used. To avoid this, make sure PID1 sets the core pattern to the empty string by default, so that this logic is disabled. This change in defaults should be useful for all systems where coredump hooks are used, as it allows useful usage of RLIMIT_CORE from these hooks again. OTOH systems that expect that coredumps are placed under the name "core" in the current directory will break with this change. Given how questionnable this behaviour is, and given that no common distro makes use of this by default it shouldn't be too much of a loss. Also, the old behaviour may be restored by explicitly configuring a "core_pattern" of "core", and setting the default system RLIMIT_CORE to 0 again via system.conf. --- src/core/main.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) (limited to 'src/core') diff --git a/src/core/main.c b/src/core/main.c index 0e6832c122..e2088574c0 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -172,19 +172,15 @@ noreturn static void crash(int sig) { if (pid < 0) log_emergency_errno(errno, "Caught <%s>, cannot fork for core dump: %m", signal_to_string(sig)); else if (pid == 0) { - struct rlimit rl = { - .rlim_cur = RLIM_INFINITY, - .rlim_max = RLIM_INFINITY, - }; - /* Enable default signal handler for core dump */ + sa = (struct sigaction) { .sa_handler = SIG_DFL, }; (void) sigaction(sig, &sa, NULL); - /* Don't limit the core dump size */ - (void) setrlimit(RLIMIT_CORE, &rl); + /* Don't limit the coredump size */ + (void) setrlimit(RLIMIT_CORE, &RLIMIT_MAKE_CONST(RLIM_INFINITY)); /* Just to be sure... */ (void) chdir("/"); @@ -1466,6 +1462,17 @@ int main(int argc, char *argv[]) { kernel_timestamp = DUAL_TIMESTAMP_NULL; } + if (getpid() == 1) { + /* Don't limit the core dump size, so that coredump handlers such as systemd-coredump (which honour the limit) + * will process core dumps for system services by default. */ + (void) setrlimit(RLIMIT_CORE, &RLIMIT_MAKE_CONST(RLIM_INFINITY)); + + /* But at the same time, turn off the core_pattern logic by default, so that no coredumps are stored + * until the systemd-coredump tool is enabled via sysctl. */ + if (!skip_setup) + (void) write_string_file("/proc/sys/kernel/core_pattern", "|/bin/false", 0); + } + /* Initialize default unit */ r = free_and_strdup(&arg_default_unit, SPECIAL_DEFAULT_TARGET); if (r < 0) { -- cgit v1.2.3-54-g00ecf From aad41f08144ab2333a3c42225c853d7d44f31c56 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 8 Feb 2016 23:54:54 +0100 Subject: core: simplify how we parse TimeoutSec=, TimeoutStartSec= and TimeoutStopSec= Let's make things more obvious by placing the parse_usec() invocation directly in config_parse_service_timeout(). --- src/core/load-fragment-gperf.gperf.m4 | 6 ++--- src/core/load-fragment.c | 50 +++++++++++++++++++---------------- 2 files changed, 30 insertions(+), 26 deletions(-) (limited to 'src/core') diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4 index c2a47c7b11..b9c67792c7 100644 --- a/src/core/load-fragment-gperf.gperf.m4 +++ b/src/core/load-fragment-gperf.gperf.m4 @@ -215,9 +215,9 @@ Service.ExecReload, config_parse_exec, SERVICE_EXE Service.ExecStop, config_parse_exec, SERVICE_EXEC_STOP, offsetof(Service, exec_command) Service.ExecStopPost, config_parse_exec, SERVICE_EXEC_STOP_POST, offsetof(Service, exec_command) Service.RestartSec, config_parse_sec, 0, offsetof(Service, restart_usec) -Service.TimeoutSec, config_parse_service_timeout, 0, offsetof(Service, timeout_start_usec) -Service.TimeoutStartSec, config_parse_service_timeout, 0, offsetof(Service, timeout_start_usec) -Service.TimeoutStopSec, config_parse_service_timeout, 0, offsetof(Service, timeout_stop_usec) +Service.TimeoutSec, config_parse_service_timeout, 0, 0 +Service.TimeoutStartSec, config_parse_service_timeout, 0, 0 +Service.TimeoutStopSec, config_parse_service_timeout, 0, 0 Service.RuntimeMaxSec, config_parse_sec, 0, offsetof(Service, runtime_max_usec) Service.WatchdogSec, config_parse_sec, 0, offsetof(Service, watchdog_usec) Service.StartLimitInterval, config_parse_sec, 0, offsetof(Unit, start_limit.interval) diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 8e5ef935f7..cd80e95598 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -1711,18 +1711,20 @@ int config_parse_bus_name( return config_parse_string(unit, filename, line, section, section_line, lvalue, ltype, k, data, userdata); } -int config_parse_service_timeout(const char *unit, - const char *filename, - unsigned line, - const char *section, - unsigned section_line, - const char *lvalue, - int ltype, - const char *rvalue, - void *data, - void *userdata) { +int config_parse_service_timeout( + const char *unit, + const char *filename, + unsigned line, + const char *section, + unsigned section_line, + const char *lvalue, + int ltype, + const char *rvalue, + void *data, + void *userdata) { Service *s = userdata; + usec_t usec; int r; assert(filename); @@ -1730,25 +1732,27 @@ int config_parse_service_timeout(const char *unit, assert(rvalue); assert(s); - r = config_parse_sec(unit, filename, line, section, section_line, lvalue, ltype, - rvalue, data, userdata); - if (r < 0) - return r; + /* This is called for three cases: TimeoutSec=, TimeoutStopSec= and TimeoutStartSec=. */ - if (streq(lvalue, "TimeoutSec")) { - s->start_timeout_defined = true; - s->timeout_stop_usec = s->timeout_start_usec; - } else if (streq(lvalue, "TimeoutStartSec")) - s->start_timeout_defined = true; + r = parse_sec(rvalue, &usec); + if (r < 0) { + log_syntax(unit, LOG_ERR, filename, line, r, "Failed to parse %s= parameter, ignoring: %s", lvalue, rvalue); + return 0; + } /* Traditionally, these options accepted 0 to disable the timeouts. However, a timeout of 0 suggests it happens * immediately, hence fix this to become USEC_INFINITY instead. This is in-line with how we internally handle * all other timeouts. */ + if (usec <= 0) + usec = USEC_INFINITY; + + if (!streq(lvalue, "TimeoutStopSec")) { + s->start_timeout_defined = true; + s->timeout_start_usec = usec; + } - if (s->timeout_start_usec <= 0) - s->timeout_start_usec = USEC_INFINITY; - if (s->timeout_stop_usec <= 0) - s->timeout_stop_usec = USEC_INFINITY; + if (!streq(lvalue, "TimeoutStartSec")) + s->timeout_stop_usec = usec; return 0; } -- cgit v1.2.3-54-g00ecf From 89beff89edba592366b2960bd830d3f6e602c2c7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 8 Feb 2016 23:56:30 +0100 Subject: core: treat JobTimeout=0 as equivalent to JobTimeout=infinity Corrects an incompatibility introduced with 36c16a7cdd6c33d7980efc2cd6a2211941f302b4. Fixes: #2537 --- man/systemd.unit.xml | 22 ++++++++------------- src/core/load-fragment-gperf.gperf.m4 | 2 +- src/core/load-fragment.c | 36 +++++++++++++++++++++++++++++++++++ src/core/load-fragment.h | 1 + 4 files changed, 46 insertions(+), 15 deletions(-) (limited to 'src/core') diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml index 2d3274bbfb..46b288f20b 100644 --- a/man/systemd.unit.xml +++ b/man/systemd.unit.xml @@ -728,20 +728,14 @@ JobTimeoutAction= JobTimeoutRebootArgument= - When a job for this unit is queued, a time-out - may be configured. If this time limit is reached, the job will - be cancelled, the unit however will not change state or even - enter the failed mode. This value defaults - to 0 (job timeouts disabled), except for device units. NB: - this timeout is independent from any unit-specific timeout - (for example, the timeout set with - TimeoutStartSec= in service units) as the - job timeout has no effect on the unit itself, only on the job - that might be pending for it. Or in other words: unit-specific - timeouts are useful to abort unit state changes, and revert - them. The job timeout set with this option however is useful - to abort only the job waiting for the unit state to - change. + When a job for this unit is queued, a time-out may be configured. If this time limit is + reached, the job will be cancelled, the unit however will not change state or even enter the + failed mode. This value defaults to infinity (job timeouts disabled), + except for device units. NB: this timeout is independent from any unit-specific timeout (for example, the + timeout set with TimeoutStartSec= in service units) as the job timeout has no effect on the + unit itself, only on the job that might be pending for it. Or in other words: unit-specific timeouts are useful + to abort unit state changes, and revert them. The job timeout set with this option however is useful to abort + only the job waiting for the unit state to change. JobTimeoutAction= optionally configures an additional diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4 index b9c67792c7..5b99398307 100644 --- a/src/core/load-fragment-gperf.gperf.m4 +++ b/src/core/load-fragment-gperf.gperf.m4 @@ -161,7 +161,7 @@ Unit.OnFailureJobMode, config_parse_job_mode, 0, Unit.OnFailureIsolate, config_parse_job_mode_isolate, 0, offsetof(Unit, on_failure_job_mode) Unit.IgnoreOnIsolate, config_parse_bool, 0, offsetof(Unit, ignore_on_isolate) Unit.IgnoreOnSnapshot, config_parse_warn_compat, DISABLED_LEGACY, 0 -Unit.JobTimeoutSec, config_parse_sec, 0, offsetof(Unit, job_timeout) +Unit.JobTimeoutSec, config_parse_sec_fix_0, 0, offsetof(Unit, job_timeout) Unit.JobTimeoutAction, config_parse_failure_action, 0, offsetof(Unit, job_timeout_action) Unit.JobTimeoutRebootArgument, config_parse_string, 0, offsetof(Unit, job_timeout_reboot_arg) Unit.StartLimitInterval, config_parse_sec, 0, offsetof(Unit, start_limit.interval) diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index cd80e95598..e0c318c110 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -1757,6 +1757,42 @@ int config_parse_service_timeout( return 0; } +int config_parse_sec_fix_0( + const char *unit, + const char *filename, + unsigned line, + const char *section, + unsigned section_line, + const char *lvalue, + int ltype, + const char *rvalue, + void *data, + void *userdata) { + + usec_t *usec = data; + int r; + + assert(filename); + assert(lvalue); + assert(rvalue); + assert(usec); + + /* This is pretty much like config_parse_sec(), except that this treats a time of 0 as infinity, for + * compatibility with older versions of systemd where 0 instead of infinity was used as indicator to turn off a + * timeout. */ + + r = parse_sec(rvalue, usec); + if (r < 0) { + log_syntax(unit, LOG_ERR, filename, line, r, "Failed to parse %s= parameter, ignoring: %s", lvalue, rvalue); + return 0; + } + + if (*usec <= 0) + *usec = USEC_INFINITY; + + return 0; +} + int config_parse_busname_service( const char *unit, const char *filename, diff --git a/src/core/load-fragment.h b/src/core/load-fragment.h index da215195cb..5fb5910919 100644 --- a/src/core/load-fragment.h +++ b/src/core/load-fragment.h @@ -109,6 +109,7 @@ int config_parse_bus_name(const char* unit, const char *filename, unsigned line, int config_parse_exec_utmp_mode(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_working_directory(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_fdname(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); +int config_parse_sec_fix_0(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); /* gperf prototypes */ const struct ConfigPerfItem* load_fragment_gperf_lookup(const char *key, unsigned length); -- cgit v1.2.3-54-g00ecf