diff --git a/.github/codeql-queries/PotentiallyDangerousFunction.ql b/.github/codeql-queries/PotentiallyDangerousFunction.ql index 63fd14e75f..d5a5635c35 100644 --- a/.github/codeql-queries/PotentiallyDangerousFunction.ql +++ b/.github/codeql-queries/PotentiallyDangerousFunction.ql @@ -42,7 +42,7 @@ predicate potentiallyDangerousFunction(Function f, string message) { message = "Call to ntohs() is confusing. Use be16toh() instead." ) or ( f.getQualifiedName() = "strerror" and - message = "Call to strerror() is not thread-safe. Use strerror_r() or printf()'s %m format string instead." + message = "Call to strerror() is not thread-safe. Use printf()'s %m format string or STRERROR() instead." ) or ( f.getQualifiedName() = "accept" and message = "Call to accept() is not O_CLOEXEC-safe. Use accept4() instead." diff --git a/man/journal-enumerate-fields.c b/man/journal-enumerate-fields.c new file mode 100644 index 0000000000..f4b6b7b078 --- /dev/null +++ b/man/journal-enumerate-fields.c @@ -0,0 +1,22 @@ +/* SPDX-License-Identifier: CC0-1.0 */ + +#include +#include +#include + +int main(int argc, char *argv[]) { + sd_journal *j; + const char *field; + int r; + + r = sd_journal_open(&j, SD_JOURNAL_LOCAL_ONLY); + if (r < 0) { + errno = -r; + fprintf(stderr, "Failed to open journal: %m\n"); + return 1; + } + SD_JOURNAL_FOREACH_FIELD(j, field) + printf("%s\n", field); + sd_journal_close(j); + return 0; +} diff --git a/man/journal-iterate-foreach.c b/man/journal-iterate-foreach.c new file mode 100644 index 0000000000..363101d5ba --- /dev/null +++ b/man/journal-iterate-foreach.c @@ -0,0 +1,31 @@ +/* SPDX-License-Identifier: CC0-1.0 */ + +#include +#include +#include + +int main(int argc, char *argv[]) { + int r; + sd_journal *j; + r = sd_journal_open(&j, SD_JOURNAL_LOCAL_ONLY); + if (r < 0) { + errno = -r; + fprintf(stderr, "Failed to open journal: %m\n"); + return 1; + } + SD_JOURNAL_FOREACH(j) { + const char *d; + size_t l; + + r = sd_journal_get_data(j, "MESSAGE", (const void **)&d, &l); + if (r < 0) { + errno = -r; + fprintf(stderr, "Failed to read message field: %m\n"); + continue; + } + + printf("%.*s\n", (int) l, d); + } + sd_journal_close(j); + return 0; +} diff --git a/man/journal-iterate-unique.c b/man/journal-iterate-unique.c index e9f273c6e5..ac0c638032 100644 --- a/man/journal-iterate-unique.c +++ b/man/journal-iterate-unique.c @@ -1,7 +1,7 @@ /* SPDX-License-Identifier: CC0-1.0 */ +#include #include -#include #include int main(int argc, char *argv[]) { @@ -12,12 +12,14 @@ int main(int argc, char *argv[]) { r = sd_journal_open(&j, SD_JOURNAL_LOCAL_ONLY); if (r < 0) { - fprintf(stderr, "Failed to open journal: %s\n", strerror(-r)); + errno = -r; + fprintf(stderr, "Failed to open journal: %m\n"); return 1; } r = sd_journal_query_unique(j, "_SYSTEMD_UNIT"); if (r < 0) { - fprintf(stderr, "Failed to query journal: %s\n", strerror(-r)); + errno = -r; + fprintf(stderr, "Failed to query journal: %m\n"); return 1; } SD_JOURNAL_FOREACH_UNIQUE(j, d, l) diff --git a/man/journal-iterate-wait.c b/man/journal-iterate-wait.c index 2b6d8a0fa8..60b3459a18 100644 --- a/man/journal-iterate-wait.c +++ b/man/journal-iterate-wait.c @@ -1,7 +1,7 @@ /* SPDX-License-Identifier: CC0-1.0 */ +#include #include -#include #include int main(int argc, char *argv[]) { @@ -9,7 +9,8 @@ int main(int argc, char *argv[]) { sd_journal *j; r = sd_journal_open(&j, SD_JOURNAL_LOCAL_ONLY); if (r < 0) { - fprintf(stderr, "Failed to open journal: %s\n", strerror(-r)); + errno = -r; + fprintf(stderr, "Failed to open journal: %m\n"); return 1; } for (;;) { @@ -17,21 +18,24 @@ int main(int argc, char *argv[]) { size_t l; r = sd_journal_next(j); if (r < 0) { - fprintf(stderr, "Failed to iterate to next entry: %s\n", strerror(-r)); + errno = -r; + fprintf(stderr, "Failed to iterate to next entry: %m\n"); break; } if (r == 0) { /* Reached the end, let's wait for changes, and try again */ r = sd_journal_wait(j, (uint64_t) -1); if (r < 0) { - fprintf(stderr, "Failed to wait for changes: %s\n", strerror(-r)); + errno = -r; + fprintf(stderr, "Failed to wait for changes: %m\n"); break; } continue; } r = sd_journal_get_data(j, "MESSAGE", &d, &l); if (r < 0) { - fprintf(stderr, "Failed to read message field: %s\n", strerror(-r)); + errno = -r; + fprintf(stderr, "Failed to read message field: %m\n"); continue; } printf("%.*s\n", (int) l, (const char*) d); diff --git a/man/journal-stream-fd.c b/man/journal-stream-fd.c new file mode 100644 index 0000000000..d59aa14c13 --- /dev/null +++ b/man/journal-stream-fd.c @@ -0,0 +1,29 @@ +/* SPDX-License-Identifier: CC0-1.0 */ + +#include +#include +#include +#include +#include +#include + +int main(int argc, char *argv[]) { + int fd; + FILE *log; + fd = sd_journal_stream_fd("test", LOG_INFO, 1); + if (fd < 0) { + errno = -fd; + fprintf(stderr, "Failed to create stream fd: %m\n"); + return 1; + } + log = fdopen(fd, "w"); + if (!log) { + fprintf(stderr, "Failed to create file object: %m\n"); + close(fd); + return 1; + } + fprintf(log, "Hello World!\n"); + fprintf(log, SD_WARNING "This is a warning!\n"); + fclose(log); + return 0; +} diff --git a/man/print-unit-path.c b/man/print-unit-path.c index 41a21aab52..ff52fd28cd 100644 --- a/man/print-unit-path.c +++ b/man/print-unit-path.c @@ -1,7 +1,7 @@ /* SPDX-License-Identifier: CC0-1.0 */ +#include #include -#include #include #include @@ -21,7 +21,8 @@ #define MEMBER "GetUnitByPID" static int log_error(int error, const char *message) { - fprintf(stderr, "%s: %s\n", message, strerror(-error)); + errno = -error; + fprintf(stderr, "%s: %m\n", message); return error; } diff --git a/man/sd-bus-errors.xml b/man/sd-bus-errors.xml index a69efe03f0..f3b1515c78 100644 --- a/man/sd-bus-errors.xml +++ b/man/sd-bus-errors.xml @@ -268,7 +268,7 @@ sd_bus_error3, sd_bus_message_set_allow_interactive_authorization3, errno3, - strerror3 + strerror_r3 diff --git a/man/sd_bus_error.xml b/man/sd_bus_error.xml index 6f5e9e4b97..b2f62f2fa0 100644 --- a/man/sd_bus_error.xml +++ b/man/sd_bus_error.xml @@ -401,7 +401,7 @@ sd-bus-errors3, sd_bus_error_add_map3, errno3, - strerror_r3 + strerror_r3 diff --git a/man/sd_bus_error_add_map.xml b/man/sd_bus_error_add_map.xml index 2976233a7c..a49b554459 100644 --- a/man/sd_bus_error_add_map.xml +++ b/man/sd_bus_error_add_map.xml @@ -132,7 +132,7 @@ sd_bus_error3, sd-bus-errors3, errno3, - strerror_r3 + strerror_r3 diff --git a/man/sd_bus_new.xml b/man/sd_bus_new.xml index 41964640b6..b45106168c 100644 --- a/man/sd_bus_new.xml +++ b/man/sd_bus_new.xml @@ -119,8 +119,10 @@ int r; … r = sd_bus_default(&bus); - if (r < 0) - fprintf(stderr, "Failed to allocate bus: %s\n", strerror(-r)); + if (r < 0) { + errno = -r; + fprintf(stderr, "Failed to allocate bus: %m\n"); + } … } diff --git a/man/sd_device_ref.xml b/man/sd_device_ref.xml index c2ba6b1571..2b8512a572 100644 --- a/man/sd_device_ref.xml +++ b/man/sd_device_ref.xml @@ -62,8 +62,10 @@ int r; … r = sd_device_new_from_syspath(&device, "…"); - if (r < 0) - fprintf(stderr, "Failed to allocate device: %s\n", strerror(-r)); + if (r < 0) { + errno = -r; + fprintf(stderr, "Failed to allocate device: %m\n"); + } … } diff --git a/man/sd_event_new.xml b/man/sd_event_new.xml index 352137ec09..8a105a764b 100644 --- a/man/sd_event_new.xml +++ b/man/sd_event_new.xml @@ -135,8 +135,10 @@ int r; … r = sd_event_default(&event); - if (r < 0) - fprintf(stderr, "Failed to allocate event loop: %s\n", strerror(-r)); + if (r < 0) { + errno = -r; + fprintf(stderr, "Failed to allocate event loop: %m\n"); + } … } diff --git a/man/sd_journal_enumerate_fields.xml b/man/sd_journal_enumerate_fields.xml index e074906980..2d8055ec89 100644 --- a/man/sd_journal_enumerate_fields.xml +++ b/man/sd_journal_enumerate_fields.xml @@ -94,26 +94,7 @@ Use the SD_JOURNAL_FOREACH_FIELD() macro to iterate through all field names in use in the current journal. - #include <stdio.h> -#include <string.h> -#include <systemd/sd-journal.h> - -int main(int argc, char *argv[]) { - sd_journal *j; - const char *field; - int r; - - r = sd_journal_open(&j, SD_JOURNAL_LOCAL_ONLY); - if (r < 0) { - fprintf(stderr, "Failed to open journal: %s\n", strerror(-r)); - return 1; - } - SD_JOURNAL_FOREACH_FIELD(j, field) - printf("%s\n", field); - sd_journal_close(j); - return 0; -} - + diff --git a/man/sd_journal_next.xml b/man/sd_journal_next.xml index e0f0a04974..628abb296c 100644 --- a/man/sd_journal_next.xml +++ b/man/sd_journal_next.xml @@ -129,34 +129,7 @@ Iterating through the journal: - #include <stdio.h> -#include <string.h> -#include <systemd/sd-journal.h> - -int main(int argc, char *argv[]) { - int r; - sd_journal *j; - r = sd_journal_open(&j, SD_JOURNAL_LOCAL_ONLY); - if (r < 0) { - fprintf(stderr, "Failed to open journal: %s\n", strerror(-r)); - return 1; - } - SD_JOURNAL_FOREACH(j) { - const char *d; - size_t l; - - r = sd_journal_get_data(j, "MESSAGE", (const void **)&d, &l); - if (r < 0) { - fprintf(stderr, "Failed to read message field: %s\n", strerror(-r)); - continue; - } - - printf("%.*s\n", (int) l, d); - } - sd_journal_close(j); - return 0; -} - + diff --git a/man/sd_journal_stream_fd.xml b/man/sd_journal_stream_fd.xml index af2234e77d..40d2804121 100644 --- a/man/sd_journal_stream_fd.xml +++ b/man/sd_journal_stream_fd.xml @@ -100,33 +100,7 @@ Creating a log stream suitable for fprintf3: - #include <syslog.h> -#include <stdio.h> -#include <string.h> -#include <unistd.h> -#include <systemd/sd-journal.h> -#include <systemd/sd-daemon.h> - -int main(int argc, char *argv[]) { - int fd; - FILE *log; - fd = sd_journal_stream_fd("test", LOG_INFO, 1); - if (fd < 0) { - fprintf(stderr, "Failed to create stream fd: %s\n", strerror(-fd)); - return 1; - } - log = fdopen(fd, "w"); - if (!log) { - fprintf(stderr, "Failed to create file object: %m\n"); - close(fd); - return 1; - } - fprintf(log, "Hello World!\n"); - fprintf(log, SD_WARNING "This is a warning!\n"); - fclose(log); - return 0; -} - + diff --git a/man/sd_login_monitor_new.xml b/man/sd_login_monitor_new.xml index 081796249d..8118121281 100644 --- a/man/sd_login_monitor_new.xml +++ b/man/sd_login_monitor_new.xml @@ -116,8 +116,10 @@ int r; … r = sd_login_monitor_new(NULL, &m); - if (r < 0) - fprintf(stderr, "Failed to allocate login monitor object: %s\n", strerror(-r)); + if (r < 0) { + errno = -r; + fprintf(stderr, "Failed to allocate login monitor object: %m\n"); + } … } diff --git a/man/sd_notify.xml b/man/sd_notify.xml index 31388b9c3d..de402950bb 100644 --- a/man/sd_notify.xml +++ b/man/sd_notify.xml @@ -424,8 +424,8 @@ sd_notifyf(0, "STATUS=Failed to start up: %s\n" "ERRNO=%i", - strerror(errno), - errno); + strerror_r(errnum, (char[1024]){}, 1024), + errnum); diff --git a/src/analyze/analyze-critical-chain.c b/src/analyze/analyze-critical-chain.c index bb077e2293..f782f95d5f 100644 --- a/src/analyze/analyze-critical-chain.c +++ b/src/analyze/analyze-critical-chain.c @@ -98,7 +98,7 @@ static int list_dependencies_one(sd_bus *bus, const char *name, unsigned level, return r; STRV_FOREACH(c, deps) { - times = hashmap_get(unit_times_hashmap, *c); /* lgtm [cpp/inconsistent-null-check] */ + times = hashmap_get(unit_times_hashmap, *c); if (times_in_range(times, boot) && times->activated >= service_longest) service_longest = times->activated; } @@ -107,7 +107,7 @@ static int list_dependencies_one(sd_bus *bus, const char *name, unsigned level, return r; STRV_FOREACH(c, deps) { - times = hashmap_get(unit_times_hashmap, *c); /* lgtm [cpp/inconsistent-null-check] */ + times = hashmap_get(unit_times_hashmap, *c); if (times_in_range(times, boot) && service_longest - times->activated <= arg_fuzz) to_print++; } @@ -116,7 +116,7 @@ static int list_dependencies_one(sd_bus *bus, const char *name, unsigned level, return r; STRV_FOREACH(c, deps) { - times = hashmap_get(unit_times_hashmap, *c); /* lgtm [cpp/inconsistent-null-check] */ + times = hashmap_get(unit_times_hashmap, *c); if (!times_in_range(times, boot) || service_longest - times->activated > arg_fuzz) continue; diff --git a/src/analyze/analyze-dot.c b/src/analyze/analyze-dot.c index 13bea4598f..bf8aa8148f 100644 --- a/src/analyze/analyze-dot.c +++ b/src/analyze/analyze-dot.c @@ -149,7 +149,7 @@ int verb_dot(int argc, char *argv[], void *userdata) { r = bus_call_method(bus, bus_systemd_mgr, "ListUnits", &error, &reply, NULL); if (r < 0) - log_error_errno(r, "Failed to list units: %s", bus_error_message(&error, r)); + return log_error_errno(r, "Failed to list units: %s", bus_error_message(&error, r)); r = sd_bus_message_enter_container(reply, SD_BUS_TYPE_ARRAY, "(ssssssouso)"); if (r < 0) diff --git a/src/analyze/analyze-verify-util.c b/src/analyze/analyze-verify-util.c index bbadd1a9f4..a646e5807c 100644 --- a/src/analyze/analyze-verify-util.c +++ b/src/analyze/analyze-verify-util.c @@ -214,7 +214,7 @@ static int verify_documentation(Unit *u, bool check_man) { } static int verify_unit(Unit *u, bool check_man, const char *root) { - _cleanup_(sd_bus_error_free) sd_bus_error err = SD_BUS_ERROR_NULL; + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; int r, k; assert(u); @@ -223,9 +223,9 @@ static int verify_unit(Unit *u, bool check_man, const char *root) { unit_dump(u, stdout, "\t"); log_unit_debug(u, "Creating %s/start job", u->id); - r = manager_add_job(u->manager, JOB_START, u, JOB_REPLACE, NULL, &err, NULL); + r = manager_add_job(u->manager, JOB_START, u, JOB_REPLACE, NULL, &error, NULL); if (r < 0) - log_unit_error_errno(u, r, "Failed to create %s/start: %s", u->id, bus_error_message(&err, r)); + log_unit_error_errno(u, r, "Failed to create %s/start: %s", u->id, bus_error_message(&error, r)); k = verify_socket(u); if (k < 0 && r == 0) diff --git a/src/basic/errno-util.h b/src/basic/errno-util.h index a14167f526..091f99c590 100644 --- a/src/basic/errno-util.h +++ b/src/basic/errno-util.h @@ -6,6 +6,21 @@ #include "macro.h" +/* strerror(3) says that glibc uses a maximum length of 1024 bytes. */ +#define ERRNO_BUF_LEN 1024 + +/* Note: the lifetime of the compound literal is the immediately surrounding block, + * see C11 §6.5.2.5, and + * https://stackoverflow.com/questions/34880638/compound-literal-lifetime-and-if-blocks + * + * Note that we use the GNU variant of strerror_r() here. */ +#define STRERROR(errnum) strerror_r(abs(errnum), (char[ERRNO_BUF_LEN]){}, ERRNO_BUF_LEN) + +/* A helper to print an error message or message for functions that return 0 on EOF. + * Note that we can't use ({ … }) to define a temporary variable, so errnum is + * evaluated twice. */ +#define STRERROR_OR_EOF(errnum) ((errnum) != 0 ? STRERROR(errnum) : "Unexpected EOF") + static inline void _reset_errno_(int *saved_errno) { if (*saved_errno < 0) /* Invalidated by UNPROTECT_ERRNO? */ return; @@ -22,6 +37,10 @@ static inline void _reset_errno_(int *saved_errno) { _saved_errno_ = -1; \ } while (false) +#define LOCAL_ERRNO(value) \ + PROTECT_ERRNO; \ + errno = abs(value) + static inline int negative_errno(void) { /* This helper should be used to shut up gcc if you know 'errno' is * negative. Instead of "return -errno;", use "return negative_errno();" @@ -54,11 +73,6 @@ static inline int RET_NERRNO(int ret) { return ret; } -static inline const char *strerror_safe(int error) { - /* 'safe' here does NOT mean thread safety. */ - return strerror(abs(error)); /* lgtm [cpp/potentially-dangerous-function] */ -} - static inline int errno_or_else(int fallback) { /* To be used when invoking library calls where errno handling is not defined clearly: we return * errno if it is set, and the specified error otherwise. The idea is that the caller initializes diff --git a/src/basic/in-addr-util.c b/src/basic/in-addr-util.c index cefe3f7661..05c729d34d 100644 --- a/src/basic/in-addr-util.c +++ b/src/basic/in-addr-util.c @@ -66,7 +66,7 @@ bool in4_addr_is_link_local_dynamic(const struct in_addr *a) { bool in6_addr_is_link_local(const struct in6_addr *a) { assert(a); - return IN6_IS_ADDR_LINKLOCAL(a); /* lgtm [cpp/potentially-dangerous-function] */ + return IN6_IS_ADDR_LINKLOCAL(a); } int in_addr_is_link_local(int family, const union in_addr_union *u) { @@ -131,7 +131,7 @@ int in_addr_is_localhost(int family, const union in_addr_union *u) { return in4_addr_is_localhost(&u->in); if (family == AF_INET6) - return IN6_IS_ADDR_LOOPBACK(&u->in6); /* lgtm [cpp/potentially-dangerous-function] */ + return IN6_IS_ADDR_LOOPBACK(&u->in6); return -EAFNOSUPPORT; } @@ -144,7 +144,7 @@ int in_addr_is_localhost_one(int family, const union in_addr_union *u) { return be32toh(u->in.s_addr) == UINT32_C(0x7F000001); if (family == AF_INET6) - return IN6_IS_ADDR_LOOPBACK(&u->in6); /* lgtm [cpp/potentially-dangerous-function] */ + return IN6_IS_ADDR_LOOPBACK(&u->in6); return -EAFNOSUPPORT; } @@ -194,8 +194,7 @@ int in_addr_prefix_intersect( assert(a); assert(b); - /* Checks whether there are any addresses that are in both - * networks */ + /* Checks whether there are any addresses that are in both networks */ m = MIN(aprefixlen, bprefixlen); @@ -203,7 +202,7 @@ int in_addr_prefix_intersect( uint32_t x, nm; x = be32toh(a->in.s_addr ^ b->in.s_addr); - nm = (m == 0) ? 0 : 0xFFFFFFFFUL << (32 - m); + nm = m == 0 ? 0 : 0xFFFFFFFFUL << (32 - m); return (x & nm) == 0; } diff --git a/src/basic/log.c b/src/basic/log.c index c5e16d669b..39d08b0928 100644 --- a/src/basic/log.c +++ b/src/basic/log.c @@ -732,14 +732,12 @@ int log_internalv( const char *format, va_list ap) { - char buffer[LINE_MAX]; - PROTECT_ERRNO; - if (_likely_(LOG_PRI(level) > log_max_level)) return -ERRNO_VALUE(error); /* Make sure that %m maps to the specified error (or "Success"). */ - errno = ERRNO_VALUE(error); + char buffer[LINE_MAX]; + LOCAL_ERRNO(ERRNO_VALUE(error)); (void) vsnprintf(buffer, sizeof buffer, format, ap); @@ -777,14 +775,13 @@ int log_object_internalv( const char *format, va_list ap) { - PROTECT_ERRNO; char *buffer, *b; if (_likely_(LOG_PRI(level) > log_max_level)) return -ERRNO_VALUE(error); /* Make sure that %m maps to the specified error (or "Success"). */ - errno = ERRNO_VALUE(error); + LOCAL_ERRNO(ERRNO_VALUE(error)); /* Prepend the object name before the message */ if (object) { @@ -1341,18 +1338,19 @@ int log_syntax_internal( const char *func, const char *format, ...) { + PROTECT_ERRNO; + if (log_syntax_callback) log_syntax_callback(unit, level, log_syntax_callback_userdata); - PROTECT_ERRNO; - char buffer[LINE_MAX]; - va_list ap; - const char *unit_fmt = NULL; - if (_likely_(LOG_PRI(level) > log_max_level) || log_target == LOG_TARGET_NULL) return -ERRNO_VALUE(error); + char buffer[LINE_MAX]; + va_list ap; + const char *unit_fmt = NULL; + errno = ERRNO_VALUE(error); va_start(ap, format); diff --git a/src/basic/process-util.c b/src/basic/process-util.c index 3dd6f4df59..fb0b38fa49 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -815,7 +815,7 @@ int wait_for_terminate_with_timeout(pid_t pid, usec_t timeout) { if (status.si_pid == pid) { /* This is the correct child. */ if (status.si_code == CLD_EXITED) - return (status.si_status == 0) ? 0 : -EPROTO; + return status.si_status == 0 ? 0 : -EPROTO; else return -EPROTO; } diff --git a/src/basic/stdio-util.h b/src/basic/stdio-util.h index 69d7062ec6..f647f125ac 100644 --- a/src/basic/stdio-util.h +++ b/src/basic/stdio-util.h @@ -9,16 +9,16 @@ #include "macro.h" #include "memory-util.h" -#define snprintf_ok(buf, len, fmt, ...) \ - ({ \ - char *_buf = (buf); \ - size_t _len = (len); \ - int _snpf = snprintf(_buf, _len, (fmt), __VA_ARGS__); \ - _snpf >= 0 && (size_t) _snpf < _len ? _buf : NULL; \ +#define snprintf_ok(buf, len, fmt, ...) \ + ({ \ + char *_buf = (buf); \ + size_t _len = (len); \ + int _snpf = snprintf(_buf, _len, (fmt), ##__VA_ARGS__); \ + _snpf >= 0 && (size_t) _snpf < _len ? _buf : NULL; \ }) #define xsprintf(buf, fmt, ...) \ - assert_message_se(snprintf_ok(buf, ELEMENTSOF(buf), fmt, __VA_ARGS__), "xsprintf: " #buf "[] must be big enough") + assert_message_se(snprintf_ok(buf, ELEMENTSOF(buf), fmt, ##__VA_ARGS__), "xsprintf: " #buf "[] must be big enough") #define VA_FORMAT_ADVANCE(format, ap) \ do { \ diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index fdbc55b884..3e3268b4d4 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -734,12 +734,12 @@ static bool menu_run( if (refresh) { for (UINTN i = idx_first; i <= idx_last && i < config->entry_count; i++) { print_at(x_start, y_start + i - idx_first, - (i == idx_highlight) ? COLOR_HIGHLIGHT : COLOR_ENTRY, + i == idx_highlight ? COLOR_HIGHLIGHT : COLOR_ENTRY, lines[i]); if (i == config->idx_default_efivar) print_at(x_start, y_start + i - idx_first, - (i == idx_highlight) ? COLOR_HIGHLIGHT : COLOR_ENTRY, + i == idx_highlight ? COLOR_HIGHLIGHT : COLOR_ENTRY, unicode_supported() ? L" ►" : L"=>"); } refresh = false; diff --git a/src/core/automount.c b/src/core/automount.c index 6c75d0e3ad..a44b8e878d 100644 --- a/src/core/automount.c +++ b/src/core/automount.c @@ -1041,7 +1041,7 @@ static int automount_dispatch_io(sd_event_source *s, int fd, uint32_t events, vo r = manager_add_job(UNIT(a)->manager, JOB_STOP, trigger, JOB_REPLACE, NULL, &error, NULL); if (r < 0) { - log_unit_warning(UNIT(a), "Failed to queue umount startup job: %s", bus_error_message(&error, r)); + log_unit_warning(UNIT(a), "Failed to queue unmount job: %s", bus_error_message(&error, r)); goto fail; } break; diff --git a/src/core/cgroup.c b/src/core/cgroup.c index cc18da1f22..c44966839c 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -398,17 +398,13 @@ static char *format_cgroup_memory_limit_comparison(char *buf, size_t l, Unit *u, * In the absence of reliably being able to detect whether memcg swap support is available or not, * only complain if the error is not ENOENT. */ if (r > 0 || IN_SET(r, -ENODATA, -EOWNERDEAD) || - (r == -ENOENT && streq(property_name, "MemorySwapMax"))) { + (r == -ENOENT && streq(property_name, "MemorySwapMax"))) buf[0] = 0; - return buf; - } - - if (r < 0) { - (void) snprintf(buf, l, " (error getting kernel value: %s)", strerror_safe(r)); - return buf; - } - - (void) snprintf(buf, l, " (different value in kernel: %" PRIu64 ")", kval); + else if (r < 0) { + errno = -r; + (void) snprintf(buf, l, " (error getting kernel value: %m)"); + } else + (void) snprintf(buf, l, " (different value in kernel: %" PRIu64 ")", kval); return buf; } diff --git a/src/core/execute.c b/src/core/execute.c index 734111bc75..b73ed8945f 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -6428,9 +6428,10 @@ static void exec_command_dump(ExecCommand *c, FILE *f, const char *prefix) { prefix2 = strjoina(prefix, "\t"); cmd = quote_command_line(c->argv, SHELL_ESCAPE_EMPTY); + fprintf(f, "%sCommand Line: %s\n", - prefix, cmd ?: strerror_safe(ENOMEM)); + prefix, strnull(cmd)); exec_status_dump(&c->exec_status, f, prefix2); } diff --git a/src/core/job.c b/src/core/job.c index 419fe54bf0..032554a0ac 100644 --- a/src/core/job.c +++ b/src/core/job.c @@ -142,7 +142,7 @@ void job_uninstall(Job *j) { job_set_state(j, JOB_WAITING); - pj = (j->type == JOB_NOP) ? &j->unit->nop_job : &j->unit->job; + pj = j->type == JOB_NOP ? &j->unit->nop_job : &j->unit->job; assert(*pj == j); /* Detach from next 'bigger' objects */ @@ -203,7 +203,7 @@ Job* job_install(Job *j) { assert(j->type < _JOB_TYPE_MAX_IN_TRANSACTION); assert(j->state == JOB_WAITING); - pj = (j->type == JOB_NOP) ? &j->unit->nop_job : &j->unit->job; + pj = j->type == JOB_NOP ? &j->unit->nop_job : &j->unit->job; uj = *pj; if (uj) { @@ -264,7 +264,7 @@ int job_install_deserialized(Job *j) { "Invalid job type %s in deserialization.", strna(job_type_to_string(j->type))); - pj = (j->type == JOB_NOP) ? &j->unit->nop_job : &j->unit->job; + pj = j->type == JOB_NOP ? &j->unit->nop_job : &j->unit->job; if (*pj) return log_unit_debug_errno(j->unit, SYNTHETIC_ERRNO(EEXIST), "Unit already has a job installed. Not installing deserialized job."); diff --git a/src/core/selinux-access.c b/src/core/selinux-access.c index 848ae246a7..c69baa8a1a 100644 --- a/src/core/selinux-access.c +++ b/src/core/selinux-access.c @@ -137,6 +137,7 @@ _printf_(2, 3) static int log_callback(int type, const char *fmt, ...) { } static int access_init(sd_bus_error *error) { + int r; if (!mac_selinux_use()) return 0; @@ -145,23 +146,20 @@ static int access_init(sd_bus_error *error) { return 1; if (avc_open(NULL, 0) != 0) { - int saved_errno = errno; - bool enforce; + r = -errno; /* Save original errno for later */ - enforce = security_getenforce() != 0; - log_full_errno(enforce ? LOG_ERR : LOG_WARNING, saved_errno, "Failed to open the SELinux AVC: %m"); + bool enforce = security_getenforce() != 0; + log_full_errno(enforce ? LOG_ERR : LOG_WARNING, r, "Failed to open the SELinux AVC: %m"); - /* If enforcement isn't on, then let's suppress this - * error, and just don't do any AVC checks. The - * warning we printed is hence all the admin will - * see. */ + /* If enforcement isn't on, then let's suppress this error, and just don't do any AVC checks. + * The warning we printed is hence all the admin will see. */ if (!enforce) return 0; - /* Return an access denied error, if we couldn't load - * the AVC but enforcing mode was on, or we couldn't - * determine whether it is one. */ - return sd_bus_error_setf(error, SD_BUS_ERROR_ACCESS_DENIED, "Failed to open the SELinux AVC: %s", strerror_safe(saved_errno)); + /* Return an access denied error based on the original errno, if we couldn't load the AVC but + * enforcing mode was on, or we couldn't determine whether it is one. */ + errno = -r; + return sd_bus_error_setf(error, SD_BUS_ERROR_ACCESS_DENIED, "Failed to open the SELinux AVC: %m"); } selinux_set_callback(SELINUX_CB_AUDIT, (union selinux_callback) { .func_audit = audit_callback }); diff --git a/src/core/socket.c b/src/core/socket.c index 308f84898c..2ac041f9d7 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -739,16 +739,14 @@ static void socket_dump(Unit *u, FILE *f, const char *prefix) { switch (p->type) { case SOCKET_SOCKET: { _cleanup_free_ char *k = NULL; - const char *t; int r; r = socket_address_print(&p->address, &k); - if (r < 0) - t = strerror_safe(r); - else - t = k; - - fprintf(f, "%s%s: %s\n", prefix, listen_lookup(socket_address_family(&p->address), p->address.type), t); + if (r < 0) { + errno = -r; + fprintf(f, "%s%s: %m\n", prefix, listen_lookup(socket_address_family(&p->address), p->address.type)); + } else + fprintf(f, "%s%s: %s\n", prefix, listen_lookup(socket_address_family(&p->address), p->address.type), k); break; } case SOCKET_SPECIAL: diff --git a/src/core/unit-serialize.c b/src/core/unit-serialize.c index e85b41a97d..21457dc6a5 100644 --- a/src/core/unit-serialize.c +++ b/src/core/unit-serialize.c @@ -848,8 +848,10 @@ void unit_dump(Unit *u, FILE *f, const char *prefix) { fprintf(f, "%s\tMerged into: %s\n", prefix, u->merged_into->id); - else if (u->load_state == UNIT_ERROR) - fprintf(f, "%s\tLoad Error Code: %s\n", prefix, strerror_safe(u->load_error)); + else if (u->load_state == UNIT_ERROR) { + errno = abs(u->load_error); + fprintf(f, "%s\tLoad Error Code: %m\n", prefix); + } for (const char *n = sd_bus_track_first(u->bus_track); n; n = sd_bus_track_next(u->bus_track)) fprintf(f, "%s\tBus Ref: %s\n", prefix, n); diff --git a/src/home/pam_systemd_home.c b/src/home/pam_systemd_home.c index 35195c2ef0..8b41416578 100644 --- a/src/home/pam_systemd_home.c +++ b/src/home/pam_systemd_home.c @@ -25,12 +25,10 @@ static int parse_argv( bool *please_suspend, bool *debug) { - int i; - assert(argc >= 0); assert(argc == 0 || argv); - for (i = 0; i < argc; i++) { + for (int i = 0; i < argc; i++) { const char *v; if ((v = startswith(argv[i], "suspend="))) { @@ -107,15 +105,11 @@ static int acquire_user_record( if (!username) { r = pam_get_user(handle, &username, NULL); - if (r != PAM_SUCCESS) { - pam_syslog(handle, LOG_ERR, "Failed to get user name: %s", pam_strerror(handle, r)); - return r; - } + if (r != PAM_SUCCESS) + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to get user name: @PAMERR@"); - if (isempty(username)) { - pam_syslog(handle, LOG_ERR, "User name not set."); - return PAM_SERVICE_ERR; - } + if (isempty(username)) + return pam_syslog_pam_error(handle, LOG_ERR, PAM_SERVICE_ERR, "User name not set."); } /* Let's bypass all IPC complexity for the two user names we know for sure we don't manage, and for @@ -135,10 +129,8 @@ static int acquire_user_record( /* Let's use the cache, so that we can share it between the session and the authentication hooks */ r = pam_get_data(handle, homed_field, (const void**) &json); - if (!IN_SET(r, PAM_SUCCESS, PAM_NO_MODULE_DATA)) { - pam_syslog(handle, LOG_ERR, "Failed to get PAM user record data: %s", pam_strerror(handle, r)); - return r; - } + if (!IN_SET(r, PAM_SUCCESS, PAM_NO_MODULE_DATA)) + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to get PAM user record data: @PAMERR@"); if (r == PAM_SUCCESS && json) { /* We determined earlier that this is not a homed user? Then exit early. (We use -1 as * negative cache indicator) */ @@ -179,11 +171,9 @@ static int acquire_user_record( return pam_log_oom(handle); r = pam_set_data(handle, homed_field, json_copy, pam_cleanup_free); - if (r != PAM_SUCCESS) { - pam_syslog(handle, LOG_ERR, "Failed to set PAM user record data '%s': %s", - homed_field, pam_strerror(handle, r)); - return r; - } + if (r != PAM_SUCCESS) + return pam_syslog_pam_error(handle, LOG_ERR, r, + "Failed to set PAM user record data '%s': @PAMERR@", homed_field); /* Take a second copy: for the generic data field, the one which we share with * pam_systemd. While we insist on only reusing homed records, pam_systemd is fine with homed @@ -197,36 +187,29 @@ static int acquire_user_record( return pam_log_oom(handle); r = pam_set_data(handle, generic_field, json_copy, pam_cleanup_free); - if (r != PAM_SUCCESS) { - pam_syslog(handle, LOG_ERR, "Failed to set PAM user record data '%s': %s", - homed_field, pam_strerror(handle, r)); - return r; - } + if (r != PAM_SUCCESS) + return pam_syslog_pam_error(handle, LOG_ERR, r, + "Failed to set PAM user record data '%s': @PAMERR@", homed_field); TAKE_PTR(json_copy); } r = json_parse(json, JSON_PARSE_SENSITIVE, &v, NULL, NULL); - if (r < 0) { - pam_syslog(handle, LOG_ERR, "Failed to parse JSON user record: %s", strerror_safe(r)); - return PAM_SERVICE_ERR; - } + if (r < 0) + return pam_syslog_errno(handle, LOG_ERR, r, "Failed to parse JSON user record: %m"); ur = user_record_new(); if (!ur) return pam_log_oom(handle); r = user_record_load(ur, v, USER_RECORD_LOAD_REFUSE_SECRET|USER_RECORD_PERMISSIVE); - if (r < 0) { - pam_syslog(handle, LOG_ERR, "Failed to load user record: %s", strerror_safe(r)); - return PAM_SERVICE_ERR; - } + if (r < 0) + return pam_syslog_errno(handle, LOG_ERR, r, "Failed to load user record: %m"); /* Safety check if cached record actually matches what we are looking for */ - if (!streq_ptr(username, ur->user_name)) { - pam_syslog(handle, LOG_ERR, "Acquired user record does not match user name."); - return PAM_SERVICE_ERR; - } + if (!streq_ptr(username, ur->user_name)) + return pam_syslog_pam_error(handle, LOG_ERR, PAM_SERVICE_ERR, + "Acquired user record does not match user name."); if (ret_record) *ret_record = TAKE_PTR(ur); @@ -237,8 +220,9 @@ user_unknown: /* Cache this, so that we don't check again */ r = pam_set_data(handle, homed_field, POINTER_MAX, NULL); if (r != PAM_SUCCESS) - pam_syslog(handle, LOG_ERR, "Failed to set PAM user record data '%s' to invalid, ignoring: %s", - homed_field, pam_strerror(handle, r)); + pam_syslog_pam_error(handle, LOG_ERR, r, + "Failed to set PAM user record data '%s' to invalid, ignoring: @PAMERR@", + homed_field); return PAM_USER_UNKNOWN; } @@ -256,7 +240,8 @@ static int release_user_record(pam_handle_t *handle, const char *username) { r = pam_set_data(handle, homed_field, NULL, NULL); if (r != PAM_SUCCESS) - pam_syslog(handle, LOG_ERR, "Failed to release PAM user record data '%s': %s", homed_field, pam_strerror(handle, r)); + pam_syslog_pam_error(handle, LOG_ERR, r, + "Failed to release PAM user record data '%s': @PAMERR@", homed_field); generic_field = strjoin("systemd-user-record-", username); if (!generic_field) @@ -264,7 +249,8 @@ static int release_user_record(pam_handle_t *handle, const char *username) { k = pam_set_data(handle, generic_field, NULL, NULL); if (k != PAM_SUCCESS) - pam_syslog(handle, LOG_ERR, "Failed to release PAM user record data '%s': %s", generic_field, pam_strerror(handle, k)); + pam_syslog_pam_error(handle, LOG_ERR, k, + "Failed to release PAM user record data '%s': @PAMERR@", generic_field); return IN_SET(r, PAM_SUCCESS, PAM_NO_MODULE_DATA) ? k : r; } @@ -288,14 +274,15 @@ static int handle_generic_user_record_error( /* Logs about all errors, except for PAM_CONV_ERR, i.e. when requesting more info failed. */ if (sd_bus_error_has_name(error, BUS_ERROR_HOME_ABSENT)) { - (void) pam_prompt(handle, PAM_ERROR_MSG, NULL, "Home of user %s is currently absent, please plug in the necessary storage device or backing file system.", user_name); - pam_syslog(handle, LOG_ERR, "Failed to acquire home for user %s: %s", user_name, bus_error_message(error, ret)); - return PAM_PERM_DENIED; + (void) pam_prompt(handle, PAM_ERROR_MSG, NULL, + "Home of user %s is currently absent, please plug in the necessary storage device or backing file system.", user_name); + return pam_syslog_pam_error(handle, LOG_ERR, PAM_PERM_DENIED, + "Failed to acquire home for user %s: %s", user_name, bus_error_message(error, ret)); } else if (sd_bus_error_has_name(error, BUS_ERROR_AUTHENTICATION_LIMIT_HIT)) { (void) pam_prompt(handle, PAM_ERROR_MSG, NULL, "Too frequent login attempts for user %s, try again later.", user_name); - pam_syslog(handle, LOG_ERR, "Failed to acquire home for user %s: %s", user_name, bus_error_message(error, ret)); - return PAM_MAXTRIES; + return pam_syslog_pam_error(handle, LOG_ERR, PAM_MAXTRIES, + "Failed to acquire home for user %s: %s", user_name, bus_error_message(error, ret)); } else if (sd_bus_error_has_name(error, BUS_ERROR_BAD_PASSWORD)) { _cleanup_(erase_and_freep) char *newp = NULL; @@ -313,16 +300,13 @@ static int handle_generic_user_record_error( if (r != PAM_SUCCESS) return PAM_CONV_ERR; /* no logging here */ - if (isempty(newp)) { - pam_syslog(handle, LOG_DEBUG, "Password request aborted."); - return PAM_AUTHTOK_ERR; - } + if (isempty(newp)) + return pam_syslog_pam_error(handle, LOG_DEBUG, PAM_AUTHTOK_ERR, + "Password request aborted."); r = user_record_set_password(secret, STRV_MAKE(newp), true); - if (r < 0) { - pam_syslog(handle, LOG_ERR, "Failed to store password: %s", strerror_safe(r)); - return PAM_SERVICE_ERR; - } + if (r < 0) + return pam_syslog_errno(handle, LOG_ERR, r, "Failed to store password: %m"); } else if (sd_bus_error_has_name(error, BUS_ERROR_BAD_RECOVERY_KEY)) { _cleanup_(erase_and_freep) char *newp = NULL; @@ -340,16 +324,13 @@ static int handle_generic_user_record_error( if (r != PAM_SUCCESS) return PAM_CONV_ERR; /* no logging here */ - if (isempty(newp)) { - pam_syslog(handle, LOG_DEBUG, "Recovery key request aborted."); - return PAM_AUTHTOK_ERR; - } + if (isempty(newp)) + return pam_syslog_pam_error(handle, LOG_DEBUG, PAM_AUTHTOK_ERR, + "Recovery key request aborted."); r = user_record_set_password(secret, STRV_MAKE(newp), true); - if (r < 0) { - pam_syslog(handle, LOG_ERR, "Failed to store recovery key: %s", strerror_safe(r)); - return PAM_SERVICE_ERR; - } + if (r < 0) + return pam_syslog_errno(handle, LOG_ERR, r, "Failed to store recovery key: %m"); } else if (sd_bus_error_has_name(error, BUS_ERROR_BAD_PASSWORD_AND_NO_TOKEN)) { _cleanup_(erase_and_freep) char *newp = NULL; @@ -366,16 +347,14 @@ static int handle_generic_user_record_error( if (r != PAM_SUCCESS) return PAM_CONV_ERR; /* no logging here */ - if (isempty(newp)) { - pam_syslog(handle, LOG_DEBUG, "Password request aborted."); - return PAM_AUTHTOK_ERR; - } + if (isempty(newp)) + return pam_syslog_pam_error(handle, LOG_DEBUG, PAM_AUTHTOK_ERR, + "Password request aborted."); + r = user_record_set_password(secret, STRV_MAKE(newp), true); - if (r < 0) { - pam_syslog(handle, LOG_ERR, "Failed to store password: %s", strerror_safe(r)); - return PAM_SERVICE_ERR; - } + if (r < 0) + return pam_syslog_errno(handle, LOG_ERR, r, "Failed to store password: %m"); } else if (sd_bus_error_has_name(error, BUS_ERROR_TOKEN_PIN_NEEDED)) { _cleanup_(erase_and_freep) char *newp = NULL; @@ -386,16 +365,12 @@ static int handle_generic_user_record_error( if (r != PAM_SUCCESS) return PAM_CONV_ERR; /* no logging here */ - if (isempty(newp)) { - pam_syslog(handle, LOG_DEBUG, "PIN request aborted."); - return PAM_AUTHTOK_ERR; - } + if (isempty(newp)) + return pam_syslog_pam_error(handle, LOG_DEBUG, PAM_AUTHTOK_ERR, "PIN request aborted."); r = user_record_set_token_pin(secret, STRV_MAKE(newp), false); - if (r < 0) { - pam_syslog(handle, LOG_ERR, "Failed to store PIN: %s", strerror_safe(r)); - return PAM_SERVICE_ERR; - } + if (r < 0) + return pam_syslog_errno(handle, LOG_ERR, r, "Failed to store PIN: %m"); } else if (sd_bus_error_has_name(error, BUS_ERROR_TOKEN_PROTECTED_AUTHENTICATION_PATH_NEEDED)) { @@ -404,10 +379,9 @@ static int handle_generic_user_record_error( (void) pam_prompt(handle, PAM_ERROR_MSG, NULL, "Please authenticate physically on security token of user %s.", user_name); r = user_record_set_pkcs11_protected_authentication_path_permitted(secret, true); - if (r < 0) { - pam_syslog(handle, LOG_ERR, "Failed to set PKCS#11 protected authentication path permitted flag: %s", strerror_safe(r)); - return PAM_SERVICE_ERR; - } + if (r < 0) + return pam_syslog_errno(handle, LOG_ERR, r, + "Failed to set PKCS#11 protected authentication path permitted flag: %m"); } else if (sd_bus_error_has_name(error, BUS_ERROR_TOKEN_USER_PRESENCE_NEEDED)) { @@ -416,10 +390,9 @@ static int handle_generic_user_record_error( (void) pam_prompt(handle, PAM_ERROR_MSG, NULL, "Please confirm presence on security token of user %s.", user_name); r = user_record_set_fido2_user_presence_permitted(secret, true); - if (r < 0) { - pam_syslog(handle, LOG_ERR, "Failed to set FIDO2 user presence permitted flag: %s", strerror_safe(r)); - return PAM_SERVICE_ERR; - } + if (r < 0) + return pam_syslog_errno(handle, LOG_ERR, r, + "Failed to set FIDO2 user presence permitted flag: %m"); } else if (sd_bus_error_has_name(error, BUS_ERROR_TOKEN_USER_VERIFICATION_NEEDED)) { @@ -428,10 +401,9 @@ static int handle_generic_user_record_error( (void) pam_prompt(handle, PAM_ERROR_MSG, NULL, "Please verify user on security token of user %s.", user_name); r = user_record_set_fido2_user_verification_permitted(secret, true); - if (r < 0) { - pam_syslog(handle, LOG_ERR, "Failed to set FIDO2 user verification permitted flag: %s", strerror_safe(r)); - return PAM_SERVICE_ERR; - } + if (r < 0) + return pam_syslog_errno(handle, LOG_ERR, r, + "Failed to set FIDO2 user verification permitted flag: %m"); } else if (sd_bus_error_has_name(error, BUS_ERROR_TOKEN_PIN_LOCKED)) { @@ -448,16 +420,12 @@ static int handle_generic_user_record_error( if (r != PAM_SUCCESS) return PAM_CONV_ERR; /* no logging here */ - if (isempty(newp)) { - pam_syslog(handle, LOG_DEBUG, "PIN request aborted."); - return PAM_AUTHTOK_ERR; - } + if (isempty(newp)) + return pam_syslog_pam_error(handle, LOG_DEBUG, PAM_AUTHTOK_ERR, "PIN request aborted."); r = user_record_set_token_pin(secret, STRV_MAKE(newp), false); - if (r < 0) { - pam_syslog(handle, LOG_ERR, "Failed to store PIN: %s", strerror_safe(r)); - return PAM_SERVICE_ERR; - } + if (r < 0) + return pam_syslog_errno(handle, LOG_ERR, r, "Failed to store PIN: %m"); } else if (sd_bus_error_has_name(error, BUS_ERROR_TOKEN_BAD_PIN_FEW_TRIES_LEFT)) { _cleanup_(erase_and_freep) char *newp = NULL; @@ -469,16 +437,12 @@ static int handle_generic_user_record_error( if (r != PAM_SUCCESS) return PAM_CONV_ERR; /* no logging here */ - if (isempty(newp)) { - pam_syslog(handle, LOG_DEBUG, "PIN request aborted."); - return PAM_AUTHTOK_ERR; - } + if (isempty(newp)) + return pam_syslog_pam_error(handle, LOG_DEBUG, PAM_AUTHTOK_ERR, "PIN request aborted."); r = user_record_set_token_pin(secret, STRV_MAKE(newp), false); - if (r < 0) { - pam_syslog(handle, LOG_ERR, "Failed to store PIN: %s", strerror_safe(r)); - return PAM_SERVICE_ERR; - } + if (r < 0) + return pam_syslog_errno(handle, LOG_ERR, r, "Failed to store PIN: %m"); } else if (sd_bus_error_has_name(error, BUS_ERROR_TOKEN_BAD_PIN_ONE_TRY_LEFT)) { _cleanup_(erase_and_freep) char *newp = NULL; @@ -490,21 +454,16 @@ static int handle_generic_user_record_error( if (r != PAM_SUCCESS) return PAM_CONV_ERR; /* no logging here */ - if (isempty(newp)) { - pam_syslog(handle, LOG_DEBUG, "PIN request aborted."); - return PAM_AUTHTOK_ERR; - } + if (isempty(newp)) + return pam_syslog_pam_error(handle, LOG_DEBUG, PAM_AUTHTOK_ERR, "PIN request aborted."); r = user_record_set_token_pin(secret, STRV_MAKE(newp), false); - if (r < 0) { - pam_syslog(handle, LOG_ERR, "Failed to store PIN: %s", strerror_safe(r)); - return PAM_SERVICE_ERR; - } + if (r < 0) + return pam_syslog_errno(handle, LOG_ERR, r, "Failed to store PIN: %m"); - } else { - pam_syslog(handle, LOG_ERR, "Failed to acquire home for user %s: %s", user_name, bus_error_message(error, ret)); - return PAM_SERVICE_ERR; - } + } else + return pam_syslog_pam_error(handle, LOG_ERR, PAM_SERVICE_ERR, + "Failed to acquire home for user %s: %s", user_name, bus_error_message(error, ret)); return PAM_SUCCESS; } @@ -536,15 +495,11 @@ static int acquire_home( * authentication if possible, but with authentication if necessary. */ r = pam_get_user(handle, &username, NULL); - if (r != PAM_SUCCESS) { - pam_syslog(handle, LOG_ERR, "Failed to get user name: %s", pam_strerror(handle, r)); - return r; - } + if (r != PAM_SUCCESS) + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to get user name: @PAMERR@"); - if (isempty(username)) { - pam_syslog(handle, LOG_ERR, "User name not set."); - return PAM_SERVICE_ERR; - } + if (isempty(username)) + return pam_syslog_pam_error(handle, LOG_ERR, PAM_SERVICE_ERR, "User name not set."); /* If we already have acquired the fd, let's shortcut this */ fd_field = strjoin("systemd-home-fd-", username); @@ -552,10 +507,9 @@ static int acquire_home( return pam_log_oom(handle); r = pam_get_data(handle, fd_field, &home_fd_ptr); - if (!IN_SET(r, PAM_SUCCESS, PAM_NO_MODULE_DATA)) { - pam_syslog(handle, LOG_ERR, "Failed to retrieve PAM home reference fd: %s", pam_strerror(handle, r)); - return r; - } + if (!IN_SET(r, PAM_SUCCESS, PAM_NO_MODULE_DATA)) + return pam_syslog_pam_error(handle, LOG_ERR, r, + "Failed to retrieve PAM home reference fd: @PAMERR@"); if (r == PAM_SUCCESS && PTR_TO_FD(home_fd_ptr) >= 0) return PAM_SUCCESS; @@ -590,17 +544,14 @@ static int acquire_home( * without anything, maybe some other authentication mechanism systemd-homed * implements (such as PKCS#11) allows us to authenticate without anything else. */ r = pam_get_item(handle, PAM_AUTHTOK, (const void**) &cached_password); - if (!IN_SET(r, PAM_BAD_ITEM, PAM_SUCCESS)) { - pam_syslog(handle, LOG_ERR, "Failed to get cached password: %s", pam_strerror(handle, r)); - return r; - } + if (!IN_SET(r, PAM_BAD_ITEM, PAM_SUCCESS)) + return pam_syslog_pam_error(handle, LOG_ERR, r, + "Failed to get cached password: @PAMERR@"); if (!isempty(cached_password)) { r = user_record_set_password(secret, STRV_MAKE(cached_password), true); - if (r < 0) { - pam_syslog(handle, LOG_ERR, "Failed to store password: %s", strerror_safe(r)); - return PAM_SERVICE_ERR; - } + if (r < 0) + return pam_syslog_errno(handle, LOG_ERR, r, "Failed to store password: %m"); } } @@ -661,18 +612,17 @@ static int acquire_home( return pam_bus_log_parse_error(handle, r); acquired_fd = fcntl(fd, F_DUPFD_CLOEXEC, 3); - if (acquired_fd < 0) { - pam_syslog(handle, LOG_ERR, "Failed to duplicate acquired fd: %s", bus_error_message(&error, r)); - return PAM_SERVICE_ERR; - } - + if (acquired_fd < 0) + return pam_syslog_errno(handle, LOG_ERR, errno, + "Failed to duplicate acquired fd: %m"); break; } if (++n_attempts >= 5) { - (void) pam_prompt(handle, PAM_ERROR_MSG, NULL, "Too many unsuccessful login attempts for user %s, refusing.", ur->user_name); - pam_syslog(handle, LOG_ERR, "Failed to acquire home for user %s: %s", ur->user_name, bus_error_message(&error, r)); - return PAM_MAXTRIES; + (void) pam_prompt(handle, PAM_ERROR_MSG, NULL, + "Too many unsuccessful login attempts for user %s, refusing.", ur->user_name); + return pam_syslog_pam_error(handle, LOG_ERR, PAM_MAXTRIES, + "Failed to acquire home for user %s: %s", ur->user_name, bus_error_message(&error, r)); } /* Try again, this time with authentication if we didn't do that before. */ @@ -682,17 +632,13 @@ static int acquire_home( /* Later PAM modules may need the auth token, but only during pam_authenticate. */ if (please_authenticate && !strv_isempty(secret->password)) { r = pam_set_item(handle, PAM_AUTHTOK, *secret->password); - if (r < 0) { - pam_syslog(handle, LOG_ERR, "Failed to set PAM auth token: %s", pam_strerror(handle, r)); - return r; - } + if (r != PAM_SUCCESS) + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to set PAM auth token: @PAMERR@"); } r = pam_set_data(handle, fd_field, FD_TO_PTR(acquired_fd), cleanup_home_fd); - if (r < 0) { - pam_syslog(handle, LOG_ERR, "Failed to set PAM bus data: %s", pam_strerror(handle, r)); - return r; - } + if (r != PAM_SUCCESS) + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to set PAM bus data: @PAMERR@"); TAKE_FD(acquired_fd); if (do_auth) { @@ -705,7 +651,6 @@ static int acquire_home( } pam_syslog(handle, LOG_NOTICE, "Home for user %s successfully acquired.", ur->user_name); - return PAM_SUCCESS; } @@ -724,16 +669,14 @@ static int release_home_fd(pam_handle_t *handle, const char *username) { r = pam_get_data(handle, fd_field, &home_fd_ptr); if (r == PAM_NO_MODULE_DATA || (r == PAM_SUCCESS && PTR_TO_FD(home_fd_ptr) < 0)) return PAM_NO_MODULE_DATA; - if (r != PAM_SUCCESS) { - pam_syslog(handle, LOG_ERR, "Failed to retrieve PAM home reference fd: %s", pam_strerror(handle, r)); - return r; - } + if (r != PAM_SUCCESS) + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to retrieve PAM home reference fd: @PAMERR@"); r = pam_set_data(handle, fd_field, NULL, NULL); if (r != PAM_SUCCESS) - pam_syslog(handle, LOG_ERR, "Failed to release PAM home reference fd: %s", pam_strerror(handle, r)); + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to release PAM home reference fd: @PAMERR@"); - return r; + return PAM_SUCCESS; } _public_ PAM_EXTERN int pam_sm_authenticate( @@ -789,16 +732,14 @@ _public_ PAM_EXTERN int pam_sm_open_session( return r; r = pam_putenv(handle, "SYSTEMD_HOME=1"); - if (r != PAM_SUCCESS) { - pam_syslog(handle, LOG_ERR, "Failed to set PAM environment variable $SYSTEMD_HOME: %s", pam_strerror(handle, r)); - return r; - } + if (r != PAM_SUCCESS) + return pam_syslog_pam_error(handle, LOG_ERR, r, + "Failed to set PAM environment variable $SYSTEMD_HOME: @PAMERR@"); r = pam_putenv(handle, suspend_please ? "SYSTEMD_HOME_SUSPEND=1" : "SYSTEMD_HOME_SUSPEND=0"); - if (r != PAM_SUCCESS) { - pam_syslog(handle, LOG_ERR, "Failed to set PAM environment variable $SYSTEMD_HOME_SUSPEND: %s", pam_strerror(handle, r)); - return r; - } + if (r != PAM_SUCCESS) + return pam_syslog_pam_error(handle, LOG_ERR, r, + "Failed to set PAM environment variable $SYSTEMD_HOME_SUSPEND: @PAMERR@"); /* Let's release the D-Bus connection, after all the session might live quite a long time, and we are * not going to process the bus connection in that time, so let's better close before the daemon @@ -829,15 +770,11 @@ _public_ PAM_EXTERN int pam_sm_close_session( pam_syslog(handle, LOG_DEBUG, "pam-systemd-homed session end"); r = pam_get_user(handle, &username, NULL); - if (r != PAM_SUCCESS) { - pam_syslog(handle, LOG_ERR, "Failed to get user name: %s", pam_strerror(handle, r)); - return r; - } + if (r != PAM_SUCCESS) + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to get user name: @PAMERR@"); - if (isempty(username)) { - pam_syslog(handle, LOG_ERR, "User name not set."); - return PAM_SERVICE_ERR; - } + if (isempty(username)) + return pam_syslog_pam_error(handle, LOG_ERR, PAM_SERVICE_ERR, "User name not set."); /* Let's explicitly drop the reference to the homed session, so that the subsequent ReleaseHome() * call will be able to do its thing. */ @@ -863,10 +800,9 @@ _public_ PAM_EXTERN int pam_sm_close_session( if (r < 0) { if (sd_bus_error_has_name(&error, BUS_ERROR_HOME_BUSY)) pam_syslog(handle, LOG_NOTICE, "Not deactivating home directory of %s, as it is still used.", username); - else { - pam_syslog(handle, LOG_ERR, "Failed to release user home: %s", bus_error_message(&error, r)); - return PAM_SESSION_ERR; - } + else + return pam_syslog_pam_error(handle, LOG_ERR, PAM_SESSION_ERR, + "Failed to release user home: %s", bus_error_message(&error, r)); } return PAM_SUCCESS; @@ -1017,35 +953,27 @@ _public_ PAM_EXTERN int pam_sm_chauthtok( /* Start with cached credentials */ r = pam_get_item(handle, PAM_OLDAUTHTOK, (const void**) &old_password); - if (!IN_SET(r, PAM_BAD_ITEM, PAM_SUCCESS)) { - pam_syslog(handle, LOG_ERR, "Failed to get old password: %s", pam_strerror(handle, r)); - return r; - } + if (!IN_SET(r, PAM_BAD_ITEM, PAM_SUCCESS)) + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to get old password: @PAMERR@"); + r = pam_get_item(handle, PAM_AUTHTOK, (const void**) &new_password); - if (!IN_SET(r, PAM_BAD_ITEM, PAM_SUCCESS)) { - pam_syslog(handle, LOG_ERR, "Failed to get cached password: %s", pam_strerror(handle, r)); - return r; - } + if (!IN_SET(r, PAM_BAD_ITEM, PAM_SUCCESS)) + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to get cached password: @PAMERR@"); if (isempty(new_password)) { /* No, it's not cached, then let's ask for the password and its verification, and cache * it. */ r = pam_get_authtok_noverify(handle, &new_password, "New password: "); - if (r != PAM_SUCCESS) { - pam_syslog(handle, LOG_ERR, "Failed to get new password: %s", pam_strerror(handle, r)); - return r; - } - if (isempty(new_password)) { - pam_syslog(handle, LOG_DEBUG, "Password request aborted."); - return PAM_AUTHTOK_ERR; - } + if (r != PAM_SUCCESS) + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to get new password: @PAMERR@"); + + if (isempty(new_password)) + return pam_syslog_pam_error(handle, LOG_DEBUG, PAM_AUTHTOK_ERR, "Password request aborted."); r = pam_get_authtok_verify(handle, &new_password, "new password: "); /* Lower case, since PAM prefixes 'Repeat' */ - if (r != PAM_SUCCESS) { - pam_syslog(handle, LOG_ERR, "Failed to get password again: %s", pam_strerror(handle, r)); - return r; - } + if (r != PAM_SUCCESS) + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to get password again: @PAMERR@"); // FIXME: pam_pwquality will ask for the password a third time. It really shouldn't do // that, and instead assume the password was already verified once when it is found to be @@ -1062,10 +990,8 @@ _public_ PAM_EXTERN int pam_sm_chauthtok( if (!isempty(old_password)) { r = user_record_set_password(old_secret, STRV_MAKE(old_password), true); - if (r < 0) { - pam_syslog(handle, LOG_ERR, "Failed to store old password: %s", strerror_safe(r)); - return PAM_SERVICE_ERR; - } + if (r < 0) + return pam_syslog_errno(handle, LOG_ERR, r, "Failed to store old password: %m"); } new_secret = user_record_new(); @@ -1073,10 +999,8 @@ _public_ PAM_EXTERN int pam_sm_chauthtok( return pam_log_oom(handle); r = user_record_set_password(new_secret, STRV_MAKE(new_password), true); - if (r < 0) { - pam_syslog(handle, LOG_ERR, "Failed to store new password: %s", strerror_safe(r)); - return PAM_SERVICE_ERR; - } + if (r < 0) + return pam_syslog_errno(handle, LOG_ERR, r, "Failed to store new password: %m"); for (;;) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; @@ -1101,16 +1025,14 @@ _public_ PAM_EXTERN int pam_sm_chauthtok( r = sd_bus_call(bus, m, HOME_SLOW_BUS_CALL_TIMEOUT_USEC, &error, NULL); if (r < 0) { r = handle_generic_user_record_error(handle, ur->user_name, old_secret, r, &error); - if (r == PAM_CONV_ERR) { - pam_syslog(handle, LOG_ERR, "Failed to prompt for password/prompt."); - return PAM_CONV_ERR; - } + if (r == PAM_CONV_ERR) + return pam_syslog_pam_error(handle, LOG_ERR, r, + "Failed to prompt for password/prompt."); if (r != PAM_SUCCESS) return r; - } else { - pam_syslog(handle, LOG_NOTICE, "Successfully changed password for user %s.", ur->user_name); - return PAM_SUCCESS; - } + } else + return pam_syslog_pam_error(handle, LOG_NOTICE, PAM_SUCCESS, + "Successfully changed password for user %s.", ur->user_name); if (++n_attempts >= 5) break; @@ -1118,6 +1040,6 @@ _public_ PAM_EXTERN int pam_sm_chauthtok( /* Try again */ }; - pam_syslog(handle, LOG_NOTICE, "Failed to change password for user %s: %m", ur->user_name); - return PAM_MAXTRIES; + return pam_syslog_pam_error(handle, LOG_NOTICE, PAM_MAXTRIES, + "Failed to change password for user %s: @PAMERR@", ur->user_name); } diff --git a/src/journal-remote/journal-gatewayd.c b/src/journal-remote/journal-gatewayd.c index 3e2a85ce29..34def4670e 100644 --- a/src/journal-remote/journal-gatewayd.c +++ b/src/journal-remote/journal-gatewayd.c @@ -256,7 +256,7 @@ static ssize_t request_reader_entries( errno = 0; k = fread(buf, 1, n, m->tmp); if (k != n) { - log_error("Failed to read from file: %s", errno != 0 ? strerror_safe(errno) : "Premature EOF"); + log_error("Failed to read from file: %s", STRERROR_OR_EOF(errno)); return MHD_CONTENT_READER_END_WITH_ERROR; } @@ -600,7 +600,7 @@ static ssize_t request_reader_fields( errno = 0; k = fread(buf, 1, n, m->tmp); if (k != n) { - log_error("Failed to read from file: %s", errno != 0 ? strerror_safe(errno) : "Premature EOF"); + log_error("Failed to read from file: %s", STRERROR_OR_EOF(errno)); return MHD_CONTENT_READER_END_WITH_ERROR; } diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c index f0d28fd48b..11de07fcfa 100644 --- a/src/journal/journalctl.c +++ b/src/journal/journalctl.c @@ -1460,7 +1460,7 @@ static int add_boot(sd_journal *j) { r = get_boots(j, NULL, &boot_id, arg_boot_offset); assert(r <= 1); if (r <= 0) { - const char *reason = (r == 0) ? "No such boot ID in journal" : strerror_safe(r); + const char *reason = (r == 0) ? "No such boot ID in journal" : STRERROR(r); if (sd_id128_is_null(arg_boot_id)) log_error("Data from the specified boot (%+i) is not available: %s", diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c index bfa9f44a83..71d7a59bda 100644 --- a/src/journal/journald-server.c +++ b/src/journal/journald-server.c @@ -1094,7 +1094,8 @@ void server_driver_message(Server *s, pid_t object_pid, const char *message_id, /* We failed to format the message. Emit a warning instead. */ char buf[LINE_MAX]; - xsprintf(buf, "MESSAGE=Entry printing failed: %s", strerror_safe(r)); + errno = -r; + xsprintf(buf, "MESSAGE=Entry printing failed: %m"); n = 3; iovec[n++] = IOVEC_MAKE_STRING("PRIORITY=4"); diff --git a/src/libsystemd-network/dhcp-packet.c b/src/libsystemd-network/dhcp-packet.c index 9748f8dd40..b2efd5246d 100644 --- a/src/libsystemd-network/dhcp-packet.c +++ b/src/libsystemd-network/dhcp-packet.c @@ -45,7 +45,7 @@ int dhcp_message_init( "hlen" (hardware address length) MUST be 0. "chaddr" (client hardware address) field MUST be zeroed. */ - message->hlen = (arp_type == ARPHRD_INFINIBAND) ? 0 : hlen; + message->hlen = arp_type == ARPHRD_INFINIBAND ? 0 : hlen; memcpy_safe(message->chaddr, chaddr, message->hlen); message->xid = htobe32(xid); diff --git a/src/libsystemd/sd-bus/bus-error.c b/src/libsystemd/sd-bus/bus-error.c index 218677eac4..4d687cfac9 100644 --- a/src/libsystemd/sd-bus/bus-error.c +++ b/src/libsystemd/sd-bus/bus-error.c @@ -494,7 +494,7 @@ _public_ int sd_bus_error_set_errno(sd_bus_error *e, int error) { *e = BUS_ERROR_FAILED; } - /* Now, fill in the message from strerror() if we can */ + /* Now, fill in the message from strerror_r() if we can */ bus_error_strerror(e, error); return -error; } @@ -555,7 +555,7 @@ _public_ int sd_bus_error_set_errnofv(sd_bus_error *e, int error, const char *fo } fail: - /* If that didn't work, use strerror() for the message */ + /* If that didn't work, use strerror_r() for the message */ bus_error_strerror(e, error); return -error; } @@ -586,19 +586,16 @@ _public_ int sd_bus_error_set_errnof(sd_bus_error *e, int error, const char *for return sd_bus_error_set_errno(e, error); } -const char *bus_error_message(const sd_bus_error *e, int error) { +const char* _bus_error_message(const sd_bus_error *e, int error, char buf[static ERRNO_BUF_LEN]) { + /* Sometimes, the D-Bus server is a little bit too verbose with + * its error messages, so let's override them here */ + if (sd_bus_error_has_name(e, SD_BUS_ERROR_ACCESS_DENIED)) + return "Access denied"; - if (e) { - /* Sometimes, the D-Bus server is a little bit too verbose with - * its error messages, so let's override them here */ - if (sd_bus_error_has_name(e, SD_BUS_ERROR_ACCESS_DENIED)) - return "Access denied"; + if (e && e->message) + return e->message; - if (e->message) - return e->message; - } - - return strerror_safe(error); + return strerror_r(abs(error), buf, ERRNO_BUF_LEN); } static bool map_ok(const sd_bus_error_map *map) { diff --git a/src/libsystemd/sd-bus/bus-error.h b/src/libsystemd/sd-bus/bus-error.h index 518493762c..c8768c9a7b 100644 --- a/src/libsystemd/sd-bus/bus-error.h +++ b/src/libsystemd/sd-bus/bus-error.h @@ -5,11 +5,17 @@ #include "sd-bus.h" +#include "errno-util.h" #include "macro.h" bool bus_error_is_dirty(sd_bus_error *e); -const char *bus_error_message(const sd_bus_error *e, int error); +const char* _bus_error_message(const sd_bus_error *e, int error, char buf[static ERRNO_BUF_LEN]); + +/* Note: the lifetime of the compound literal is the immediately surrounding block, + * see C11 §6.5.2.5, and + * https://stackoverflow.com/questions/34880638/compound-literal-lifetime-and-if-blocks */ +#define bus_error_message(e, error) _bus_error_message(e, error, (char[ERRNO_BUF_LEN]){}) #define BUS_ERROR_OOM SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_NO_MEMORY, "Out of memory") #define BUS_ERROR_FAILED SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_FAILED, "Operation failed") diff --git a/src/libsystemd/sd-bus/test-bus-chat.c b/src/libsystemd/sd-bus/test-bus-chat.c index df6dd62151..93e8ebfb1b 100644 --- a/src/libsystemd/sd-bus/test-bus-chat.c +++ b/src/libsystemd/sd-bus/test-bus-chat.c @@ -308,7 +308,7 @@ static void* client1(void *p) { errno = 0; if (read(pp[0], &x, 1) <= 0) { - log_error("Failed to read from pipe: %s", errno != 0 ? strerror_safe(errno) : "early read"); + log_error("Failed to read from pipe: %s", STRERROR_OR_EOF(errno)); goto finish; } diff --git a/src/libsystemd/sd-bus/test-bus-error.c b/src/libsystemd/sd-bus/test-bus-error.c index cc59de12b4..a55f3f9856 100644 --- a/src/libsystemd/sd-bus/test-bus-error.c +++ b/src/libsystemd/sd-bus/test-bus-error.c @@ -99,7 +99,7 @@ TEST(error) { assert_se(!sd_bus_error_is_set(&error)); assert_se(sd_bus_error_set_errno(&error, EBUSY) == -EBUSY); assert_se(streq(error.name, "System.Error.EBUSY")); - assert_se(streq(error.message, strerror_safe(EBUSY))); + assert_se(streq(error.message, STRERROR(EBUSY))); assert_se(sd_bus_error_has_name(&error, "System.Error.EBUSY")); assert_se(sd_bus_error_get_errno(&error) == EBUSY); assert_se(sd_bus_error_is_set(&error)); diff --git a/src/libsystemd/sd-journal/journal-verify.c b/src/libsystemd/sd-journal/journal-verify.c index 8b2c468a0b..ad4039dee0 100644 --- a/src/libsystemd/sd-journal/journal-verify.c +++ b/src/libsystemd/sd-journal/journal-verify.c @@ -67,7 +67,7 @@ static uint64_t scale_progress(uint64_t scale, uint64_t p, uint64_t m) { * Currently all callers use m >= 1, but we keep the check to be defensive. */ - if (p >= m || m == 0) // lgtm[cpp/constant-comparison] + if (p >= m || m == 0) return scale; return scale * p / m; diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c index 43c72da11f..d8ad424bfe 100644 --- a/src/login/logind-seat.c +++ b/src/login/logind-seat.c @@ -389,11 +389,11 @@ int seat_read_active_vt(Seat *s) { if (lseek(s->manager->console_active_fd, SEEK_SET, 0) < 0) return log_error_errno(errno, "lseek on console_active_fd failed: %m"); + errno = 0; k = read(s->manager->console_active_fd, t, sizeof(t)-1); - if (k <= 0) { - log_error("Failed to read current console: %s", k < 0 ? strerror_safe(errno) : "EOF"); - return k < 0 ? -errno : -EIO; - } + if (k <= 0) + return log_error_errno(errno ?: EIO, + "Failed to read current console: %s", STRERROR_OR_EOF(errno)); t[k] = 0; truncate_nl(t); diff --git a/src/login/logind-session-device.c b/src/login/logind-session-device.c index acf0f20d85..4f934dd01d 100644 --- a/src/login/logind-session-device.c +++ b/src/login/logind-session-device.c @@ -49,7 +49,7 @@ static int session_device_notify(SessionDevice *sd, enum SessionDeviceNotificati sd->session->manager->bus, &m, path, "org.freedesktop.login1.Session", - (type == SESSION_DEVICE_RESUME) ? "ResumeDevice" : "PauseDevice"); + type == SESSION_DEVICE_RESUME ? "ResumeDevice" : "PauseDevice"); if (!m) return r; diff --git a/src/login/pam_systemd.c b/src/login/pam_systemd.c index faaf6c9a69..a288b3602a 100644 --- a/src/login/pam_systemd.c +++ b/src/login/pam_systemd.c @@ -107,15 +107,11 @@ static int acquire_user_record( assert(handle); r = pam_get_user(handle, &username, NULL); - if (r != PAM_SUCCESS) { - pam_syslog(handle, LOG_ERR, "Failed to get user name: %s", pam_strerror(handle, r)); - return r; - } + if (r != PAM_SUCCESS) + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to get user name: @PAMERR@"); - if (isempty(username)) { - pam_syslog(handle, LOG_ERR, "User name not valid."); - return PAM_SERVICE_ERR; - } + if (isempty(username)) + return pam_syslog_pam_error(handle, LOG_ERR, PAM_SERVICE_ERR, "User name not valid."); /* If pam_systemd_homed (or some other module) already acquired the user record we can reuse it * here. */ @@ -124,66 +120,53 @@ static int acquire_user_record( return pam_log_oom(handle); r = pam_get_data(handle, field, (const void**) &json); - if (!IN_SET(r, PAM_SUCCESS, PAM_NO_MODULE_DATA)) { - pam_syslog(handle, LOG_ERR, "Failed to get PAM user record data: %s", pam_strerror(handle, r)); - return r; - } + if (!IN_SET(r, PAM_SUCCESS, PAM_NO_MODULE_DATA)) + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to get PAM user record data: @PAMERR@"); if (r == PAM_SUCCESS && json) { _cleanup_(json_variant_unrefp) JsonVariant *v = NULL; /* Parse cached record */ r = json_parse(json, JSON_PARSE_SENSITIVE, &v, NULL, NULL); - if (r < 0) { - pam_syslog(handle, LOG_ERR, "Failed to parse JSON user record: %s", strerror_safe(r)); - return PAM_SERVICE_ERR; - } + if (r < 0) + return pam_syslog_errno(handle, LOG_ERR, r, "Failed to parse JSON user record: %m"); ur = user_record_new(); if (!ur) return pam_log_oom(handle); r = user_record_load(ur, v, USER_RECORD_LOAD_REFUSE_SECRET|USER_RECORD_PERMISSIVE); - if (r < 0) { - pam_syslog(handle, LOG_ERR, "Failed to load user record: %s", strerror_safe(r)); - return PAM_SERVICE_ERR; - } + if (r < 0) + return pam_syslog_errno(handle, LOG_ERR, r, "Failed to load user record: %m"); /* Safety check if cached record actually matches what we are looking for */ - if (!streq_ptr(username, ur->user_name)) { - pam_syslog(handle, LOG_ERR, "Acquired user record does not match user name."); - return PAM_SERVICE_ERR; - } + if (!streq_ptr(username, ur->user_name)) + return pam_syslog_pam_error(handle, LOG_ERR, PAM_SERVICE_ERR, + "Acquired user record does not match user name."); } else { _cleanup_free_ char *formatted = NULL; /* Request the record ourselves */ r = userdb_by_name(username, 0, &ur); if (r < 0) { - pam_syslog(handle, LOG_ERR, "Failed to get user record: %s", strerror_safe(r)); + pam_syslog_errno(handle, LOG_ERR, r, "Failed to get user record: %m"); return PAM_USER_UNKNOWN; } r = json_variant_format(ur->json, 0, &formatted); - if (r < 0) { - pam_syslog(handle, LOG_ERR, "Failed to format user JSON: %s", strerror_safe(r)); - return PAM_SERVICE_ERR; - } + if (r < 0) + return pam_syslog_errno(handle, LOG_ERR, r, "Failed to format user JSON: %m"); /* And cache it for everyone else */ r = pam_set_data(handle, field, formatted, pam_cleanup_free); - if (r < 0) { - pam_syslog(handle, LOG_ERR, "Failed to set PAM user record data '%s': %s", - field, pam_strerror(handle, r)); - return r; - } - + if (r != PAM_SUCCESS) + return pam_syslog_pam_error(handle, LOG_ERR, r, + "Failed to set PAM user record data '%s': @PAMERR@", field); TAKE_PTR(formatted); } - if (!uid_is_valid(ur->uid)) { - pam_syslog(handle, LOG_ERR, "Acquired user record does not have a UID."); - return PAM_SERVICE_ERR; - } + if (!uid_is_valid(ur->uid)) + return pam_syslog_pam_error(handle, LOG_ERR, PAM_SERVICE_ERR, + "Acquired user record does not have a UID."); if (ret_record) *ret_record = TAKE_PTR(ur); @@ -326,10 +309,8 @@ static int export_legacy_dbus_address( return pam_log_oom(handle); r = pam_misc_setenv(handle, "DBUS_SESSION_BUS_ADDRESS", t, 0); - if (r != PAM_SUCCESS) { - pam_syslog(handle, LOG_ERR, "Failed to set bus variable: %s", pam_strerror(handle, r)); - return r; - } + if (r != PAM_SUCCESS) + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to set bus variable: @PAMERR@"); return PAM_SUCCESS; } @@ -471,9 +452,10 @@ static int update_environment(pam_handle_t *handle, const char *key, const char r = pam_misc_setenv(handle, key, value, 0); if (r != PAM_SUCCESS) - pam_syslog(handle, LOG_ERR, "Failed to set environment variable %s: %s", key, pam_strerror(handle, r)); + return pam_syslog_pam_error(handle, LOG_ERR, r, + "Failed to set environment variable %s: @PAMERR@", key); - return r; + return PAM_SUCCESS; } static bool validate_runtime_directory(pam_handle_t *handle, const char *path, uid_t uid) { @@ -495,7 +477,7 @@ static bool validate_runtime_directory(pam_handle_t *handle, const char *path, u } if (lstat(path, &st) < 0) { - pam_syslog(handle, LOG_ERR, "Failed to stat() runtime directory '%s': %s", path, strerror_safe(errno)); + pam_syslog_errno(handle, LOG_ERR, errno, "Failed to stat() runtime directory '%s': %m", path); goto fail; } @@ -523,10 +505,9 @@ static int pam_putenv_and_log(pam_handle_t *handle, const char *e, bool debug) { assert(e); r = pam_putenv(handle, e); - if (r != PAM_SUCCESS) { - pam_syslog(handle, LOG_ERR, "Failed to set PAM environment variable %s: %s", e, pam_strerror(handle, r)); - return r; - } + if (r != PAM_SUCCESS) + return pam_syslog_pam_error(handle, LOG_ERR, r, + "Failed to set PAM environment variable %s: @PAMERR@", e); if (debug) pam_syslog(handle, LOG_DEBUG, "PAM environment variable %s set based on user record.", e); @@ -627,9 +608,11 @@ static int apply_user_record_settings(pam_handle_t *handle, UserRecord *ur, bool if (nice_is_valid(ur->nice_level)) { if (nice(ur->nice_level) < 0) - pam_syslog(handle, LOG_ERR, "Failed to set nice level to %i, ignoring: %s", ur->nice_level, strerror_safe(errno)); + pam_syslog_errno(handle, LOG_ERR, errno, + "Failed to set nice level to %i, ignoring: %m", ur->nice_level); else if (debug) - pam_syslog(handle, LOG_DEBUG, "Nice level set, based on user record."); + pam_syslog(handle, LOG_DEBUG, + "Nice level set to %i, based on user record.", ur->nice_level); } for (int rl = 0; rl < _RLIMIT_MAX; rl++) { @@ -639,9 +622,11 @@ static int apply_user_record_settings(pam_handle_t *handle, UserRecord *ur, bool r = setrlimit_closest(rl, ur->rlimits[rl]); if (r < 0) - pam_syslog(handle, LOG_ERR, "Failed to set resource limit %s, ignoring: %s", rlimit_to_string(rl), strerror_safe(r)); + pam_syslog_errno(handle, LOG_ERR, r, + "Failed to set resource limit %s, ignoring: %m", rlimit_to_string(rl)); else if (debug) - pam_syslog(handle, LOG_DEBUG, "Resource limit %s set, based on user record.", rlimit_to_string(rl)); + pam_syslog(handle, LOG_DEBUG, + "Resource limit %s set, based on user record.", rlimit_to_string(rl)); } return PAM_SUCCESS; @@ -662,10 +647,8 @@ static int configure_runtime_directory( return PAM_SUCCESS; r = pam_misc_setenv(handle, "XDG_RUNTIME_DIR", rt, 0); - if (r != PAM_SUCCESS) { - pam_syslog(handle, LOG_ERR, "Failed to set runtime dir: %s", pam_strerror(handle, r)); - return r; - } + if (r != PAM_SUCCESS) + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to set runtime dir: @PAMERR@"); return export_legacy_dbus_address(handle, rt); } @@ -721,10 +704,8 @@ _public_ PAM_EXTERN int pam_sm_open_session( * leave. */ r = pam_get_item(handle, PAM_SERVICE, (const void**) &service); - if (!IN_SET(r, PAM_BAD_ITEM, PAM_SUCCESS)) { - pam_syslog(handle, LOG_ERR, "Failed to get PAM service: %s", pam_strerror(handle, r)); - return r; - } + if (!IN_SET(r, PAM_BAD_ITEM, PAM_SUCCESS)) + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to get PAM service: @PAMERR@"); if (streq_ptr(service, "systemd-user")) { char rt[STRLEN("/run/user/") + DECIMAL_STR_MAX(uid_t)]; @@ -739,25 +720,17 @@ _public_ PAM_EXTERN int pam_sm_open_session( /* Otherwise, we ask logind to create a session for us */ r = pam_get_item(handle, PAM_XDISPLAY, (const void**) &display); - if (!IN_SET(r, PAM_BAD_ITEM, PAM_SUCCESS)) { - pam_syslog(handle, LOG_ERR, "Failed to get PAM XDISPLAY: %s", pam_strerror(handle, r)); - return r; - } + if (!IN_SET(r, PAM_BAD_ITEM, PAM_SUCCESS)) + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to get PAM_XDISPLAY: @PAMERR@"); r = pam_get_item(handle, PAM_TTY, (const void**) &tty); - if (!IN_SET(r, PAM_BAD_ITEM, PAM_SUCCESS)) { - pam_syslog(handle, LOG_ERR, "Failed to get PAM TTY: %s", pam_strerror(handle, r)); - return r; - } + if (!IN_SET(r, PAM_BAD_ITEM, PAM_SUCCESS)) + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to get PAM_TTY: @PAMERR@"); r = pam_get_item(handle, PAM_RUSER, (const void**) &remote_user); - if (!IN_SET(r, PAM_BAD_ITEM, PAM_SUCCESS)) { - pam_syslog(handle, LOG_ERR, "Failed to get PAM RUSER: %s", pam_strerror(handle, r)); - return r; - } + if (!IN_SET(r, PAM_BAD_ITEM, PAM_SUCCESS)) + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to get PAM_RUSER: @PAMERR@"); r = pam_get_item(handle, PAM_RHOST, (const void**) &remote_host); - if (!IN_SET(r, PAM_BAD_ITEM, PAM_SUCCESS)) { - pam_syslog(handle, LOG_ERR, "Failed to get PAM RHOST: %s", pam_strerror(handle, r)); - return r; - } + if (!IN_SET(r, PAM_BAD_ITEM, PAM_SUCCESS)) + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to get PAM_RHOST: @PAMERR@"); seat = getenv_harder(handle, "XDG_SEAT", NULL); cvtnr = getenv_harder(handle, "XDG_VTNR", NULL); @@ -825,30 +798,20 @@ _public_ PAM_EXTERN int pam_sm_open_session( remote = !isempty(remote_host) && !is_localhost(remote_host); r = pam_get_data(handle, "systemd.memory_max", (const void **)&memory_max); - if (!IN_SET(r, PAM_SUCCESS, PAM_NO_MODULE_DATA)) { - pam_syslog(handle, LOG_ERR, "Failed to get PAM systemd.memory_max data: %s", pam_strerror(handle, r)); - return r; - } + if (!IN_SET(r, PAM_SUCCESS, PAM_NO_MODULE_DATA)) + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to get PAM systemd.memory_max data: @PAMERR@"); r = pam_get_data(handle, "systemd.tasks_max", (const void **)&tasks_max); - if (!IN_SET(r, PAM_SUCCESS, PAM_NO_MODULE_DATA)) { - pam_syslog(handle, LOG_ERR, "Failed to get PAM systemd.tasks_max data: %s", pam_strerror(handle, r)); - return r; - } + if (!IN_SET(r, PAM_SUCCESS, PAM_NO_MODULE_DATA)) + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to get PAM systemd.tasks_max data: @PAMERR@"); r = pam_get_data(handle, "systemd.cpu_weight", (const void **)&cpu_weight); - if (!IN_SET(r, PAM_SUCCESS, PAM_NO_MODULE_DATA)) { - pam_syslog(handle, LOG_ERR, "Failed to get PAM systemd.cpu_weight data: %s", pam_strerror(handle, r)); - return r; - } + if (!IN_SET(r, PAM_SUCCESS, PAM_NO_MODULE_DATA)) + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to get PAM systemd.cpu_weight data: @PAMERR@"); r = pam_get_data(handle, "systemd.io_weight", (const void **)&io_weight); - if (!IN_SET(r, PAM_SUCCESS, PAM_NO_MODULE_DATA)) { - pam_syslog(handle, LOG_ERR, "Failed to get PAM systemd.io_weight data: %s", pam_strerror(handle, r)); - return r; - } + if (!IN_SET(r, PAM_SUCCESS, PAM_NO_MODULE_DATA)) + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to get PAM systemd.io_weight data: @PAMERR@"); r = pam_get_data(handle, "systemd.runtime_max_sec", (const void **)&runtime_max_sec); - if (!IN_SET(r, PAM_SUCCESS, PAM_NO_MODULE_DATA)) { - pam_syslog(handle, LOG_ERR, "Failed to get PAM systemd.runtime_max_sec data: %s", pam_strerror(handle, r)); - return r; - } + if (!IN_SET(r, PAM_SUCCESS, PAM_NO_MODULE_DATA)) + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to get PAM systemd.runtime_max_sec data: @PAMERR@"); /* Talk to logind over the message bus */ @@ -994,24 +957,18 @@ _public_ PAM_EXTERN int pam_sm_open_session( } r = pam_set_data(handle, "systemd.existing", INT_TO_PTR(!!existing), NULL); - if (r != PAM_SUCCESS) { - pam_syslog(handle, LOG_ERR, "Failed to install existing flag: %s", pam_strerror(handle, r)); - return r; - } + if (r != PAM_SUCCESS) + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to install existing flag: @PAMERR@"); if (session_fd >= 0) { - session_fd = fcntl(session_fd, F_DUPFD_CLOEXEC, 3); - if (session_fd < 0) { - pam_syslog(handle, LOG_ERR, "Failed to dup session fd: %m"); - return PAM_SESSION_ERR; - } + _cleanup_close_ int fd = fcntl(session_fd, F_DUPFD_CLOEXEC, 3); + if (fd < 0) + return pam_syslog_errno(handle, LOG_ERR, errno, "Failed to dup session fd: %m"); - r = pam_set_data(handle, "systemd.session-fd", FD_TO_PTR(session_fd), NULL); - if (r != PAM_SUCCESS) { - pam_syslog(handle, LOG_ERR, "Failed to install session fd: %s", pam_strerror(handle, r)); - safe_close(session_fd); - return r; - } + r = pam_set_data(handle, "systemd.session-fd", FD_TO_PTR(fd), NULL); + if (r != PAM_SUCCESS) + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to install session fd: @PAMERR@"); + TAKE_FD(fd); } success: @@ -1052,10 +1009,9 @@ _public_ PAM_EXTERN int pam_sm_close_session( /* Only release session if it wasn't pre-existing when we * tried to create it */ r = pam_get_data(handle, "systemd.existing", &existing); - if (!IN_SET(r, PAM_SUCCESS, PAM_NO_MODULE_DATA)) { - pam_syslog(handle, LOG_ERR, "Failed to get PAM systemd.existing data: %s", pam_strerror(handle, r)); - return r; - } + if (!IN_SET(r, PAM_SUCCESS, PAM_NO_MODULE_DATA)) + return pam_syslog_pam_error(handle, LOG_ERR, r, + "Failed to get PAM systemd.existing data: @PAMERR@"); id = pam_getenv(handle, "XDG_SESSION_ID"); if (id && !existing) { @@ -1070,10 +1026,9 @@ _public_ PAM_EXTERN int pam_sm_close_session( return r; r = bus_call_method(bus, bus_login_mgr, "ReleaseSession", &error, NULL, "s", id); - if (r < 0) { - pam_syslog(handle, LOG_ERR, "Failed to release session: %s", bus_error_message(&error, r)); - return PAM_SESSION_ERR; - } + if (r < 0) + return pam_syslog_pam_error(handle, LOG_ERR, PAM_SESSION_ERR, + "Failed to release session: %s", bus_error_message(&error, r)); } /* Note that we are knowingly leaking the FIFO fd here. This way, logind can watch us die. If we diff --git a/src/network/networkd-state-file.c b/src/network/networkd-state-file.c index 4932184991..0800e91202 100644 --- a/src/network/networkd-state-file.c +++ b/src/network/networkd-state-file.c @@ -204,7 +204,7 @@ int manager_save(Manager *m) { const char *domainname; char **domains = NULL; - target_domains = (link->network->dhcp_use_domains == DHCP_USE_DOMAINS_YES) ? &search_domains : &route_domains; + target_domains = link->network->dhcp_use_domains == DHCP_USE_DOMAINS_YES ? &search_domains : &route_domains; r = sd_dhcp_lease_get_domainname(link->dhcp_lease, &domainname); if (r >= 0) { r = ordered_set_put_strdup(target_domains, domainname); diff --git a/src/resolve/resolved-dns-transaction.h b/src/resolve/resolved-dns-transaction.h index ab86f0f01f..5a0e35704c 100644 --- a/src/resolve/resolved-dns-transaction.h +++ b/src/resolve/resolved-dns-transaction.h @@ -213,8 +213,8 @@ DnsTransactionSource dns_transaction_source_from_string(const char *s) _pure_; /* Maximum attempts to send MDNS requests, see RFC 6762 Section 8.1 */ #define MDNS_TRANSACTION_ATTEMPTS_MAX 3 -#define TRANSACTION_ATTEMPTS_MAX(p) (((p) == DNS_PROTOCOL_LLMNR) ? \ - LLMNR_TRANSACTION_ATTEMPTS_MAX : \ - (((p) == DNS_PROTOCOL_MDNS) ? \ - MDNS_TRANSACTION_ATTEMPTS_MAX : \ - DNS_TRANSACTION_ATTEMPTS_MAX)) +#define TRANSACTION_ATTEMPTS_MAX(p) ((p) == DNS_PROTOCOL_LLMNR ? \ + LLMNR_TRANSACTION_ATTEMPTS_MAX : \ + (p) == DNS_PROTOCOL_MDNS ? \ + MDNS_TRANSACTION_ATTEMPTS_MAX : \ + DNS_TRANSACTION_ATTEMPTS_MAX) diff --git a/src/shared/journal-importer.c b/src/shared/journal-importer.c index 66481da267..d9eabec886 100644 --- a/src/shared/journal-importer.c +++ b/src/shared/journal-importer.c @@ -426,11 +426,10 @@ int journal_importer_push_data(JournalImporter *imp, const char *data, size_t si assert(imp->state != IMPORTER_STATE_EOF); if (!realloc_buffer(imp, imp->filled + size)) - return log_error_errno(SYNTHETIC_ERRNO(ENOMEM), + return log_error_errno(ENOMEM, "Failed to store received data of size %zu " - "(in addition to existing %zu bytes with %zu filled): %s", - size, MALLOC_SIZEOF_SAFE(imp->buf), imp->filled, - strerror_safe(ENOMEM)); + "(in addition to existing %zu bytes with %zu filled): %m", + size, MALLOC_SIZEOF_SAFE(imp->buf), imp->filled); memcpy(imp->buf + imp->filled, data, size); imp->filled += size; diff --git a/src/shared/pam-util.c b/src/shared/pam-util.c index 621e7fe802..9d74e08a2b 100644 --- a/src/shared/pam-util.c +++ b/src/shared/pam-util.c @@ -8,23 +8,45 @@ #include "errno-util.h" #include "macro.h" #include "pam-util.h" +#include "stdio-util.h" +#include "string-util.h" -int pam_log_oom(pam_handle_t *handle) { - /* This is like log_oom(), but uses PAM logging */ - pam_syslog(handle, LOG_ERR, "Out of memory."); - return PAM_BUF_ERR; +int pam_syslog_errno(pam_handle_t *handle, int level, int error, const char *format, ...) { + va_list ap; + + LOCAL_ERRNO(error); + + va_start(ap, format); + pam_vsyslog(handle, LOG_ERR, format, ap); + va_end(ap); + + return error == -ENOMEM ? PAM_BUF_ERR : PAM_SERVICE_ERR; } -int pam_bus_log_create_error(pam_handle_t *handle, int r) { - /* This is like bus_log_create_error(), but uses PAM logging */ - pam_syslog(handle, LOG_ERR, "Failed to create bus message: %s", strerror_safe(r)); - return PAM_BUF_ERR; -} +int pam_syslog_pam_error(pam_handle_t *handle, int level, int error, const char *format, ...) { + /* This wraps pam_syslog() but will replace @PAMERR@ with a string from pam_strerror(). + * @PAMERR@ must be at the very end. */ -int pam_bus_log_parse_error(pam_handle_t *handle, int r) { - /* This is like bus_log_parse_error(), but uses PAM logging */ - pam_syslog(handle, LOG_ERR, "Failed to parse bus message: %s", strerror_safe(r)); - return PAM_BUF_ERR; + va_list ap; + va_start(ap, format); + + const char *p = endswith(format, "@PAMERR@"); + if (p) { + const char *pamerr = pam_strerror(handle, error); + if (strchr(pamerr, '%')) + pamerr = "n/a"; /* We cannot have any formatting chars */ + + char buf[p - format + strlen(pamerr) + 1]; + xsprintf(buf, "%*s%s", (int)(p - format), format, pamerr); + DISABLE_WARNING_FORMAT_NONLITERAL; + pam_vsyslog(handle, level, buf, ap); + REENABLE_WARNING; + } else + pam_vsyslog(handle, level, format, ap); + + va_end(ap); + + return error; } static void cleanup_system_bus(pam_handle_t *handle, void *data, int error_status) { @@ -44,22 +66,16 @@ int pam_acquire_bus_connection(pam_handle_t *handle, sd_bus **ret) { *ret = sd_bus_ref(TAKE_PTR(bus)); /* Increase the reference counter, so that the PAM data stays valid */ return PAM_SUCCESS; } - if (!IN_SET(r, PAM_SUCCESS, PAM_NO_MODULE_DATA)) { - pam_syslog(handle, LOG_ERR, "Failed to get bus connection: %s", pam_strerror(handle, r)); - return r; - } + if (!IN_SET(r, PAM_SUCCESS, PAM_NO_MODULE_DATA)) + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to get bus connection: @PAMERR@"); r = sd_bus_open_system(&bus); - if (r < 0) { - pam_syslog(handle, LOG_ERR, "Failed to connect to system bus: %s", strerror_safe(r)); - return PAM_SERVICE_ERR; - } + if (r < 0) + return pam_syslog_errno(handle, LOG_ERR, r, "Failed to connect to system bus: %m"); r = pam_set_data(handle, "systemd-system-bus", bus, cleanup_system_bus); - if (r != PAM_SUCCESS) { - pam_syslog(handle, LOG_ERR, "Failed to set PAM bus data: %s", pam_strerror(handle, r)); - return r; - } + if (r != PAM_SUCCESS) + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to set PAM bus data: @PAMERR@"); sd_bus_ref(bus); *ret = TAKE_PTR(bus); @@ -72,9 +88,9 @@ int pam_release_bus_connection(pam_handle_t *handle) { r = pam_set_data(handle, "systemd-system-bus", NULL, NULL); if (r != PAM_SUCCESS) - pam_syslog(handle, LOG_ERR, "Failed to release PAM user record data: %s", pam_strerror(handle, r)); + return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to release PAM user record data: @PAMERR@"); - return r; + return PAM_SUCCESS; } void pam_cleanup_free(pam_handle_t *handle, void *data, int error_status) { diff --git a/src/shared/pam-util.h b/src/shared/pam-util.h index 41f1835b3c..1a17ea18c5 100644 --- a/src/shared/pam-util.h +++ b/src/shared/pam-util.h @@ -5,9 +5,24 @@ #include "sd-bus.h" -int pam_log_oom(pam_handle_t *handle); -int pam_bus_log_create_error(pam_handle_t *handle, int r); -int pam_bus_log_parse_error(pam_handle_t *handle, int r); +int pam_syslog_errno(pam_handle_t *handle, int level, int error, const char *format, ...) _printf_(4,5); + +int pam_syslog_pam_error(pam_handle_t *handle, int level, int error, const char *format, ...) _printf_(4,5); + +static inline int pam_log_oom(pam_handle_t *handle) { + /* This is like log_oom(), but uses PAM logging */ + return pam_syslog_errno(handle, LOG_ERR, ENOMEM, "Out of memory."); +} + +static inline int pam_bus_log_create_error(pam_handle_t *handle, int r) { + /* This is like bus_log_create_error(), but uses PAM logging */ + return pam_syslog_errno(handle, LOG_ERR, r, "Failed to create bus message: %m"); +} + +static inline int pam_bus_log_parse_error(pam_handle_t *handle, int r) { + /* This is like bus_log_parse_error(), but uses PAM logging */ + return pam_syslog_errno(handle, LOG_ERR, r, "Failed to parse bus message: %m"); +} int pam_acquire_bus_connection(pam_handle_t *handle, sd_bus **ret); int pam_release_bus_connection(pam_handle_t *handle); diff --git a/src/systemctl/systemctl-show.c b/src/systemctl/systemctl-show.c index 4c7094678c..8d3db98c0a 100644 --- a/src/systemctl/systemctl-show.c +++ b/src/systemctl/systemctl-show.c @@ -670,8 +670,10 @@ static void print_status_info( if (i->status_text) printf(" Status: \"%s\"\n", i->status_text); - if (i->status_errno > 0) - printf(" Error: %i (%s)\n", i->status_errno, strerror_safe(i->status_errno)); + if (i->status_errno > 0) { + errno = i->status_errno; + printf(" Error: %i (%m)\n", i->status_errno); + } if (i->ip_ingress_bytes != UINT64_MAX && i->ip_egress_bytes != UINT64_MAX) printf(" IP: %s in, %s out\n", diff --git a/src/systemd/sd-daemon.h b/src/systemd/sd-daemon.h index 442b0ac604..53a1d7ccfe 100644 --- a/src/systemd/sd-daemon.h +++ b/src/systemd/sd-daemon.h @@ -260,8 +260,8 @@ int sd_notify(int unset_environment, const char *state); sd_notifyf(0, "STATUS=Failed to start up: %s\n" "ERRNO=%i", - strerror(errno), - errno); + strerror_r(errnum, (char[1024]){}, 1024), + errnum); See sd_notifyf(3) for more information. */ diff --git a/src/test/meson.build b/src/test/meson.build index 31ac149b96..86fc1d4fc0 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -615,6 +615,8 @@ tests += [ [files('test-errno-list.c') + generated_gperf_headers], + [files('test-errno-util.c')], + [files('test-ip-protocol-list.c') + shared_generated_gperf_headers], diff --git a/src/test/test-calendarspec.c b/src/test/test-calendarspec.c index 8a0684ed66..564983b699 100644 --- a/src/test/test-calendarspec.c +++ b/src/test/test-calendarspec.c @@ -27,7 +27,7 @@ static void _test_one(int line, const char *input, const char *output) { u = now(CLOCK_REALTIME); r = calendar_spec_next_usec(c, u, &u); - log_info("Next: %s", r < 0 ? strerror_safe(r) : FORMAT_TIMESTAMP(u)); + log_info("Next: %s", r < 0 ? STRERROR(r) : FORMAT_TIMESTAMP(u)); calendar_spec_free(c); assert_se(calendar_spec_from_string(p, &c) >= 0); @@ -60,7 +60,7 @@ static void _test_next(int line, const char *input, const char *new_tz, usec_t a u = after; r = calendar_spec_next_usec(c, after, &u); - log_info("At: %s", r < 0 ? strerror_safe(r) : FORMAT_TIMESTAMP_STYLE(u, TIMESTAMP_US)); + log_info("At: %s", r < 0 ? STRERROR(r) : FORMAT_TIMESTAMP_STYLE(u, TIMESTAMP_US)); if (expect != USEC_INFINITY) assert_se(r >= 0 && u == expect); else @@ -104,10 +104,10 @@ TEST(hourly_bug_4031) { assert_se((r = calendar_spec_next_usec(c, n, &u)) >= 0); log_info("Now: %s (%"PRIu64")", FORMAT_TIMESTAMP_STYLE(n, TIMESTAMP_US), n); - log_info("Next hourly: %s (%"PRIu64")", r < 0 ? strerror_safe(r) : FORMAT_TIMESTAMP_STYLE(u, TIMESTAMP_US), u); + log_info("Next hourly: %s (%"PRIu64")", r < 0 ? STRERROR(r) : FORMAT_TIMESTAMP_STYLE(u, TIMESTAMP_US), u); assert_se((r = calendar_spec_next_usec(c, u, &w)) >= 0); - log_info("Next hourly: %s (%"PRIu64")", r < 0 ? strerror_safe(r) : FORMAT_TIMESTAMP_STYLE(w, TIMESTAMP_US), w); + log_info("Next hourly: %s (%"PRIu64")", r < 0 ? STRERROR(r) : FORMAT_TIMESTAMP_STYLE(w, TIMESTAMP_US), w); assert_se(n < u); assert_se(u <= n + USEC_PER_HOUR); diff --git a/src/test/test-errno-util.c b/src/test/test-errno-util.c new file mode 100644 index 0000000000..f858927c92 --- /dev/null +++ b/src/test/test-errno-util.c @@ -0,0 +1,50 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "errno-util.h" +#include "stdio-util.h" +#include "string-util.h" +#include "tests.h" + +TEST(strerror_not_threadsafe) { + /* Just check that strerror really is not thread-safe. */ + log_info("strerror(%d) → %s", 200, strerror(200)); + log_info("strerror(%d) → %s", 201, strerror(201)); + log_info("strerror(%d) → %s", INT_MAX, strerror(INT_MAX)); + + log_info("strerror(%d), strerror(%d) → %p, %p", 200, 201, strerror(200), strerror(201)); + + /* This call is not allowed, because the first returned string becomes invalid when + * we call strerror the second time: + * + * log_info("strerror(%d), strerror(%d) → %s, %s", 200, 201, strerror(200), strerror(201)); + */ +} + +TEST(STRERROR) { + /* Just check that STRERROR really is thread-safe. */ + log_info("STRERROR(%d) → %s", 200, STRERROR(200)); + log_info("STRERROR(%d) → %s", 201, STRERROR(201)); + log_info("STRERROR(%d), STRERROR(%d) → %s, %s", 200, 201, STRERROR(200), STRERROR(201)); + + const char *a = STRERROR(200), *b = STRERROR(201); + assert_se(strstr(a, "200")); + assert_se(strstr(b, "201")); + + /* Check with negative values */ + assert_se(streq(a, STRERROR(-200))); + assert_se(streq(b, STRERROR(-201))); + + const char *c = STRERROR(INT_MAX); + char buf[DECIMAL_STR_MAX(int)]; + xsprintf(buf, "%d", INT_MAX); /* INT_MAX is hexadecimal, use printf to convert to decimal */ + log_info("STRERROR(%d) → %s", INT_MAX, c); + assert_se(strstr(c, buf)); +} + +TEST(STRERROR_OR_ELSE) { + log_info("STRERROR_OR_ELSE(0, \"EOF\") → %s", STRERROR_OR_EOF(0)); + log_info("STRERROR_OR_ELSE(EPERM, \"EOF\") → %s", STRERROR_OR_EOF(EPERM)); + log_info("STRERROR_OR_ELSE(-EPERM, \"EOF\") → %s", STRERROR_OR_EOF(-EPERM)); +} + +DEFINE_TEST_MAIN(LOG_INFO); diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c index e3fcbd4513..f398761982 100644 --- a/src/test/test-path-util.c +++ b/src/test/test-path-util.c @@ -806,10 +806,10 @@ static void test_path_extract_filename_one(const char *input, const char *output int r; r = path_extract_filename(input, &k); - log_info_errno(r, "%s → %s/%m [expected: %s/%s]", - strnull(input), - strnull(k), /* strerror(r) is printed via %m, to avoid that the two strerror()'s overwrite each other's buffers */ - strnull(output), ret < 0 ? strerror_safe(ret) : "-"); + log_info("%s → %s/%s [expected: %s/%s]", + strnull(input), + strnull(k), r < 0 ? STRERROR(r) : "-", + strnull(output), ret < 0 ? STRERROR(ret) : "-"); assert_se(streq_ptr(k, output)); assert_se(r == ret); } @@ -850,10 +850,10 @@ static void test_path_extract_directory_one(const char *input, const char *outpu int r; r = path_extract_directory(input, &k); - log_info_errno(r, "%s → %s/%m [expected: %s/%s]", - strnull(input), - strnull(k), /* we output strerror_safe(r) via %m here, since otherwise the error buffer might be overwritten twice */ - strnull(output), strerror_safe(ret)); + log_info("%s → %s/%s [expected: %s/%s]", + strnull(input), + strnull(k), r < 0 ? STRERROR(r) : "-", + strnull(output), STRERROR(ret)); assert_se(streq_ptr(k, output)); assert_se(r == ret); diff --git a/src/test/test-sleep.c b/src/test/test-sleep.c index 6bac115f71..799e93dced 100644 --- a/src/test/test-sleep.c +++ b/src/test/test-sleep.c @@ -109,13 +109,13 @@ TEST(sleep) { log_info("/= high-level sleep verbs =/"); r = can_sleep(SLEEP_SUSPEND); - log_info("Suspend configured and possible: %s", r >= 0 ? yes_no(r) : strerror_safe(r)); + log_info("Suspend configured and possible: %s", r >= 0 ? yes_no(r) : STRERROR(r)); r = can_sleep(SLEEP_HIBERNATE); - log_info("Hibernation configured and possible: %s", r >= 0 ? yes_no(r) : strerror_safe(r)); + log_info("Hibernation configured and possible: %s", r >= 0 ? yes_no(r) : STRERROR(r)); r = can_sleep(SLEEP_HYBRID_SLEEP); - log_info("Hybrid-sleep configured and possible: %s", r >= 0 ? yes_no(r) : strerror_safe(r)); + log_info("Hybrid-sleep configured and possible: %s", r >= 0 ? yes_no(r) : STRERROR(r)); r = can_sleep(SLEEP_SUSPEND_THEN_HIBERNATE); - log_info("Suspend-then-Hibernate configured and possible: %s", r >= 0 ? yes_no(r) : strerror_safe(r)); + log_info("Suspend-then-Hibernate configured and possible: %s", r >= 0 ? yes_no(r) : STRERROR(r)); } static int intro(void) { diff --git a/src/test/test-tmpfile-util.c b/src/test/test-tmpfile-util.c index 002bd46162..415f185a81 100644 --- a/src/test/test-tmpfile-util.c +++ b/src/test/test-tmpfile-util.c @@ -13,7 +13,9 @@ static void test_tempfn_random_one(const char *p, const char *extra, const char int r; r = tempfn_random(p, extra, &s); - log_info_errno(r, "%s+%s → %s vs. %s (%i/%m vs. %i/%s)", p, strna(extra), strna(s), strna(expect), r, ret, strerror_safe(ret)); + log_info("%s+%s → %s vs. %s (%i/%s vs. %i/%s)", + p, strna(extra), strna(s), strna(expect), + r, STRERROR(r), ret, STRERROR(ret)); assert_se(!s == !expect); if (s) { @@ -89,7 +91,9 @@ static void test_tempfn_xxxxxx_one(const char *p, const char *extra, const char int r; r = tempfn_xxxxxx(p, extra, &s); - log_info_errno(r, "%s+%s → %s vs. %s (%i/%m vs. %i/%s)", p, strna(extra), strna(s), strna(expect), r, ret, strerror_safe(ret)); + log_info("%s+%s → %s vs. %s (%i/%s vs. %i/%s)", + p, strna(extra), strna(s), strna(expect), + r, STRERROR(r), ret, STRERROR(ret)); assert_se(!s == !expect); if (s) { @@ -164,7 +168,9 @@ static void test_tempfn_random_child_one(const char *p, const char *extra, const int r; r = tempfn_random_child(p, extra, &s); - log_info_errno(r, "%s+%s → %s vs. %s (%i/%m vs. %i/%s)", p, strna(extra), strna(s), strna(expect), r, ret, strerror_safe(ret)); + log_info_errno(r, "%s+%s → %s vs. %s (%i/%s vs. %i/%s)", + p, strna(extra), strna(s), strna(expect), + r, STRERROR(r), ret, STRERROR(ret)); assert_se(!s == !expect); if (s) {