diff --git a/src/basic/log.c b/src/basic/log.c index 88aea4e1da..72b60da6c6 100644 --- a/src/basic/log.c +++ b/src/basic/log.c @@ -83,6 +83,16 @@ static bool prohibit_ipc = false; * use here. */ static char *log_abort_msg = NULL; +/* An assert to use in logging functions that does not call recursively + * into our logging functions (since that might lead to a loop). */ +#define assert_raw(expr) \ + do { \ + if (_unlikely_(!(expr))) { \ + fputs(#expr "\n", stderr); \ + abort(); \ + } \ + } while (false) + static void log_close_console(void) { if (console_fd < 0) @@ -365,7 +375,7 @@ static int write_to_console( highlight = LOG_PRI(level) <= LOG_ERR && show_color; if (show_location) { - xsprintf(location, "(%s:%i) ", file, line); + (void) snprintf(location, sizeof location, "(%s:%i) ", file, line); iovec[n++] = IOVEC_MAKE_STRING(location); } @@ -498,38 +508,40 @@ static int log_do_header( const char *file, int line, const char *func, const char *object_field, const char *object, const char *extra_field, const char *extra) { + int r; - snprintf(header, size, - "PRIORITY=%i\n" - "SYSLOG_FACILITY=%i\n" - "%s%s%s" - "%s%.*i%s" - "%s%s%s" - "%s%.*i%s" - "%s%s%s" - "%s%s%s" - "SYSLOG_IDENTIFIER=%s\n", - LOG_PRI(level), - LOG_FAC(level), - isempty(file) ? "" : "CODE_FILE=", - isempty(file) ? "" : file, - isempty(file) ? "" : "\n", - line ? "CODE_LINE=" : "", - line ? 1 : 0, line, /* %.0d means no output too, special case for 0 */ - line ? "\n" : "", - isempty(func) ? "" : "CODE_FUNC=", - isempty(func) ? "" : func, - isempty(func) ? "" : "\n", - error ? "ERRNO=" : "", - error ? 1 : 0, error, - error ? "\n" : "", - isempty(object) ? "" : object_field, - isempty(object) ? "" : object, - isempty(object) ? "" : "\n", - isempty(extra) ? "" : extra_field, - isempty(extra) ? "" : extra, - isempty(extra) ? "" : "\n", - program_invocation_short_name); + r = snprintf(header, size, + "PRIORITY=%i\n" + "SYSLOG_FACILITY=%i\n" + "%s%.256s%s" /* CODE_FILE */ + "%s%.*i%s" /* CODE_LINE */ + "%s%.256s%s" /* CODE_FUNC */ + "%s%.*i%s" /* ERRNO */ + "%s%.256s%s" /* object */ + "%s%.256s%s" /* extra */ + "SYSLOG_IDENTIFIER=%.256s\n", + LOG_PRI(level), + LOG_FAC(level), + isempty(file) ? "" : "CODE_FILE=", + isempty(file) ? "" : file, + isempty(file) ? "" : "\n", + line ? "CODE_LINE=" : "", + line ? 1 : 0, line, /* %.0d means no output too, special case for 0 */ + line ? "\n" : "", + isempty(func) ? "" : "CODE_FUNC=", + isempty(func) ? "" : func, + isempty(func) ? "" : "\n", + error ? "ERRNO=" : "", + error ? 1 : 0, error, + error ? "\n" : "", + isempty(object) ? "" : object_field, + isempty(object) ? "" : object, + isempty(object) ? "" : "\n", + isempty(extra) ? "" : extra_field, + isempty(extra) ? "" : extra, + isempty(extra) ? "" : "\n", + program_invocation_short_name); + assert_raw((size_t) r < size); return 0; } @@ -577,11 +589,11 @@ int log_dispatch_internal( const char *func, const char *object_field, const char *object, - const char *extra, const char *extra_field, + const char *extra, char *buffer) { - assert(buffer); + assert_raw(buffer); if (error < 0) error = -error; @@ -698,7 +710,7 @@ int log_internalv_realm( if (error != 0) errno = error; - vsnprintf(buffer, sizeof(buffer), format, ap); + (void) vsnprintf(buffer, sizeof buffer, format, ap); return log_dispatch_internal(level, error, file, line, func, NULL, NULL, NULL, NULL, buffer); } @@ -721,7 +733,8 @@ int log_internal_realm( return r; } -int log_object_internalv( +_printf_(10,0) +static int log_object_internalv( int level, int error, const char *file, @@ -757,7 +770,7 @@ int log_object_internalv( } else b = buffer = newa(char, LINE_MAX); - vsnprintf(b, LINE_MAX, format, ap); + (void) vsnprintf(b, LINE_MAX, format, ap); return log_dispatch_internal(level, error, file, line, func, object_field, object, extra_field, extra, buffer); @@ -800,7 +813,7 @@ static void log_assert( return; DISABLE_WARNING_FORMAT_NONLITERAL; - xsprintf(buffer, format, text, file, line, func); + (void) snprintf(buffer, sizeof buffer, format, text, file, line, func); REENABLE_WARNING; log_abort_msg = buffer; @@ -974,7 +987,7 @@ int log_struct_internal( errno = error; va_copy(aq, ap); - vsnprintf(buf, sizeof(buf), format, aq); + (void) vsnprintf(buf, sizeof buf, format, aq); va_end(aq); if (startswith(buf, "MESSAGE=")) { @@ -1273,7 +1286,7 @@ int log_syntax_internal( errno = error; va_start(ap, format); - vsnprintf(buffer, sizeof(buffer), format, ap); + (void) vsnprintf(buffer, sizeof buffer, format, ap); va_end(ap); if (unit) diff --git a/src/basic/log.h b/src/basic/log.h index 5b5a25bd6d..efcf0f1bfc 100644 --- a/src/basic/log.h +++ b/src/basic/log.h @@ -144,19 +144,6 @@ int log_object_internal( const char *extra, const char *format, ...) _printf_(10,11); -int log_object_internalv( - int level, - int error, - const char *file, - int line, - const char *func, - const char *object_field, - const char *object, - const char *extra_field, - const char *extra, - const char *format, - va_list ap) _printf_(10,0); - int log_struct_internal( int level, int error, diff --git a/src/basic/stdio-util.h b/src/basic/stdio-util.h index dbfafba269..d3fed365d8 100644 --- a/src/basic/stdio-util.h +++ b/src/basic/stdio-util.h @@ -27,9 +27,11 @@ #include "macro.h" -#define xsprintf(buf, fmt, ...) \ - assert_message_se((size_t) snprintf(buf, ELEMENTSOF(buf), fmt, __VA_ARGS__) < ELEMENTSOF(buf), "xsprintf: " #buf "[] must be big enough") +#define snprintf_ok(buf, len, fmt, ...) \ + ((size_t) snprintf(buf, len, fmt, __VA_ARGS__) < (len)) +#define xsprintf(buf, fmt, ...) \ + 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/core/unit.c b/src/core/unit.c index 61a7203744..cbb3015590 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -1699,7 +1699,7 @@ static void unit_status_log_starting_stopping_reloading(Unit *u, JobType t) { format = unit_get_status_message_format(u, t); DISABLE_WARNING_FORMAT_NONLITERAL; - xsprintf(buf, format, unit_description(u)); + snprintf(buf, sizeof buf, format, unit_description(u)); REENABLE_WARNING; mid = t == JOB_START ? "MESSAGE_ID=" SD_MESSAGE_UNIT_STARTING_STR : diff --git a/src/journal/journald-native.h b/src/journal/journald-native.h index 7ca17ec609..f1b4abc41a 100644 --- a/src/journal/journald-native.h +++ b/src/journal/journald-native.h @@ -22,8 +22,6 @@ #include "journald-server.h" -bool valid_user_field(const char *p, size_t l, bool allow_protected); - void server_process_native_message(Server *s, const void *buffer, size_t buffer_size, const struct ucred *ucred, const struct timeval *tv, const char *label, size_t label_len); void server_process_native_file(Server *s, int fd, const struct ucred *ucred, const struct timeval *tv, const char *label, size_t label_len); diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 01cce28038..3e5c785660 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -2942,7 +2942,8 @@ static int start_unit_one( log_error("Failed to %s %s: %s", verb, name, bus_error_message(error, r)); if (!sd_bus_error_has_name(error, BUS_ERROR_NO_SUCH_UNIT) && - !sd_bus_error_has_name(error, BUS_ERROR_UNIT_MASKED)) + !sd_bus_error_has_name(error, BUS_ERROR_UNIT_MASKED) && + !sd_bus_error_has_name(error, BUS_ERROR_JOB_TYPE_NOT_APPLICABLE)) log_error("See %s logs and 'systemctl%s status%s %s' for details.", arg_scope == UNIT_FILE_SYSTEM ? "system" : "user", arg_scope == UNIT_FILE_SYSTEM ? "" : " --user", diff --git a/src/test/test-log.c b/src/test/test-log.c index fd19899480..4a3c8955e3 100644 --- a/src/test/test-log.c +++ b/src/test/test-log.c @@ -35,19 +35,18 @@ assert_cc((LOG_REALM_PLUS_LEVEL(LOG_REALM_SYSTEMD, LOG_LOCAL3 | LOG_DEBUG) & LOG assert_cc((LOG_REALM_PLUS_LEVEL(LOG_REALM_UDEV, LOG_USER | LOG_INFO) & LOG_PRIMASK) == LOG_INFO); -int main(int argc, char* argv[]) { - - log_set_target(LOG_TARGET_CONSOLE); - log_open(); +#define X10(x) x x x x x x x x x x +#define X100(x) X10(X10(x)) +#define X1000(x) X100(X10(x)) +static void test_log_console(void) { log_struct(LOG_INFO, "MESSAGE=Waldo PID="PID_FMT, getpid_cached(), "SERVICE=piepapo", NULL); +} - log_set_target(LOG_TARGET_JOURNAL); - log_open(); - +static void test_log_journal(void) { log_struct(LOG_INFO, "MESSAGE=Foobar PID="PID_FMT, getpid_cached(), "SERVICE=foobar", @@ -59,6 +58,32 @@ int main(int argc, char* argv[]) { (int) 1, 'A', (short) 2, (long int) 3, (long long int) 4, (void*) 1, "foo", (float) 2.5f, (double) 3.5, (long double) 4.5, "SUFFIX=GOT IT", NULL); +} + +static void test_long_lines(void) { + log_object_internal(LOG_NOTICE, + EUCLEAN, + X1000("abcd_") ".txt", + 1000000, + X1000("fff") "unc", + "OBJECT=", + X1000("obj_") "ect", + "EXTRA=", + X1000("ext_") "tra", + "asdfasdf %s asdfasdfa", "foobar"); +} + +int main(int argc, char* argv[]) { + int target; + + for (target = 0; target < _LOG_TARGET_MAX; target++) { + log_set_target(target); + log_open(); + + test_log_console(); + test_log_journal(); + test_long_lines(); + } return 0; } diff --git a/src/udev/collect/collect.c b/src/udev/collect/collect.c index 3e278bd637..2821640e93 100644 --- a/src/udev/collect/collect.c +++ b/src/udev/collect/collect.c @@ -94,7 +94,7 @@ static int prepare(char *dir, char *filename) if (r < 0 && errno != EEXIST) return -errno; - xsprintf(buf, "%s/%s", dir, filename); + snprintf(buf, sizeof buf, "%s/%s", dir, filename); fd = open(buf, O_RDWR|O_CREAT|O_CLOEXEC, S_IRUSR|S_IWUSR); if (fd < 0) diff --git a/src/udev/udev-builtin-net_id.c b/src/udev/udev-builtin-net_id.c index 945585d82a..36994360c7 100644 --- a/src/udev/udev-builtin-net_id.c +++ b/src/udev/udev-builtin-net_id.c @@ -115,6 +115,7 @@ #include "stdio-util.h" #include "string-util.h" #include "udev.h" +#include "udev-util.h" #define ONBOARD_INDEX_MAX (16*1024-1) @@ -236,11 +237,11 @@ static int dev_pci_slot(struct udev_device *dev, struct netnames *names) { size_t l; char *s; const char *attr, *port_name; - struct udev_device *pci = NULL; + _cleanup_udev_device_unref_ struct udev_device *pci = NULL; char slots[PATH_MAX]; _cleanup_closedir_ DIR *dir = NULL; struct dirent *dent; - int hotplug_slot = 0, err = 0; + int hotplug_slot = 0; if (sscanf(udev_device_get_sysname(names->pcidev), "%x:%x:%x.%u", &domain, &bus, &slot, &func) != 4) return -ENOENT; @@ -270,21 +271,20 @@ static int dev_pci_slot(struct udev_device *dev, struct netnames *names) { /* ACPI _SUN — slot user number */ pci = udev_device_new_from_subsystem_sysname(udev, "subsystem", "pci"); - if (!pci) { - err = -ENOENT; - goto out; - } + if (!pci) + return -ENOENT; + + if (!snprintf_ok(slots, sizeof slots, "%s/slots", udev_device_get_syspath(pci))) + return -ENAMETOOLONG; - xsprintf(slots, "%s/slots", udev_device_get_syspath(pci)); dir = opendir(slots); - if (!dir) { - err = -errno; - goto out; - } + if (!dir) + return -errno; FOREACH_DIRENT_ALL(dent, dir, break) { int i; - char *rest, *address, str[PATH_MAX]; + char *rest, str[PATH_MAX]; + _cleanup_free_ char *address = NULL; if (dent->d_name[0] == '.') continue; @@ -294,13 +294,11 @@ static int dev_pci_slot(struct udev_device *dev, struct netnames *names) { if (i < 1) continue; - xsprintf(str, "%s/%s/address", slots, dent->d_name); - if (read_one_line_file(str, &address) >= 0) { + if (snprintf_ok(str, sizeof str, "%s/%s/address", slots, dent->d_name) && + read_one_line_file(str, &address) >= 0) /* match slot address with device by stripping the function */ - if (strneq(address, udev_device_get_sysname(names->pcidev), strlen(address))) + if (streq(address, udev_device_get_sysname(names->pcidev))) hotplug_slot = i; - free(address); - } if (hotplug_slot > 0) break; @@ -321,9 +319,8 @@ static int dev_pci_slot(struct udev_device *dev, struct netnames *names) { if (l == 0) names->pci_slot[0] = '\0'; } -out: - udev_device_unref(pci); - return err; + + return 0; } static int names_vio(struct udev_device *dev, struct netnames *names) { @@ -517,7 +514,6 @@ static int names_ccw(struct udev_device *dev, struct netnames *names) { const char *bus_id, *subsys; size_t bus_id_len; size_t bus_id_start; - int rc; assert(dev); assert(names); @@ -559,9 +555,9 @@ static int names_ccw(struct udev_device *dev, struct netnames *names) { bus_id += bus_id_start < bus_id_len ? bus_id_start : bus_id_len - 1; /* Store the CCW bus-ID for use as network device name */ - rc = snprintf(names->ccw_busid, sizeof(names->ccw_busid), "c%s", bus_id); - if (rc >= 0 && rc < (int)sizeof(names->ccw_busid)) + if (snprintf_ok(names->ccw_busid, sizeof(names->ccw_busid), "c%s", bus_id)) names->type = NET_CCW; + return 0; } @@ -674,7 +670,7 @@ static int builtin_net_id(struct udev_device *dev, int argc, char *argv[], bool if (err >= 0 && names.type == NET_CCW) { char str[IFNAMSIZ]; - if (snprintf(str, sizeof(str), "%s%s", prefix, names.ccw_busid) < (int)sizeof(str)) + if (snprintf_ok(str, sizeof str, "%s%s", prefix, names.ccw_busid)) udev_builtin_add_property(dev, test, "ID_NET_NAME_PATH", str); goto out; } @@ -684,7 +680,7 @@ static int builtin_net_id(struct udev_device *dev, int argc, char *argv[], bool if (err >= 0 && names.type == NET_VIO) { char str[IFNAMSIZ]; - if (snprintf(str, sizeof(str), "%s%s", prefix, names.vio_slot) < (int)sizeof(str)) + if (snprintf_ok(str, sizeof str, "%s%s", prefix, names.vio_slot)) udev_builtin_add_property(dev, test, "ID_NET_NAME_SLOT", str); goto out; } @@ -694,7 +690,7 @@ static int builtin_net_id(struct udev_device *dev, int argc, char *argv[], bool if (err >= 0 && names.type == NET_PLATFORM) { char str[IFNAMSIZ]; - if (snprintf(str, sizeof(str), "%s%s", prefix, names.platform_path) < (int)sizeof(str)) + if (snprintf_ok(str, sizeof str, "%s%s", prefix, names.platform_path)) udev_builtin_add_property(dev, test, "ID_NET_NAME_PATH", str); goto out; } @@ -708,21 +704,21 @@ static int builtin_net_id(struct udev_device *dev, int argc, char *argv[], bool if (names.type == NET_PCI) { char str[IFNAMSIZ]; - if (names.pci_onboard[0]) - if (snprintf(str, sizeof(str), "%s%s", prefix, names.pci_onboard) < (int)sizeof(str)) - udev_builtin_add_property(dev, test, "ID_NET_NAME_ONBOARD", str); + if (names.pci_onboard[0] && + snprintf_ok(str, sizeof str, "%s%s", prefix, names.pci_onboard)) + udev_builtin_add_property(dev, test, "ID_NET_NAME_ONBOARD", str); - if (names.pci_onboard_label) - if (snprintf(str, sizeof(str), "%s%s", prefix, names.pci_onboard_label) < (int)sizeof(str)) - udev_builtin_add_property(dev, test, "ID_NET_LABEL_ONBOARD", str); + if (names.pci_onboard_label && + snprintf_ok(str, sizeof str, "%s%s", prefix, names.pci_onboard_label)) + udev_builtin_add_property(dev, test, "ID_NET_LABEL_ONBOARD", str); - if (names.pci_path[0]) - if (snprintf(str, sizeof(str), "%s%s", prefix, names.pci_path) < (int)sizeof(str)) - udev_builtin_add_property(dev, test, "ID_NET_NAME_PATH", str); + if (names.pci_path[0] && + snprintf_ok(str, sizeof str, "%s%s", prefix, names.pci_path)) + udev_builtin_add_property(dev, test, "ID_NET_NAME_PATH", str); - if (names.pci_slot[0]) - if (snprintf(str, sizeof(str), "%s%s", prefix, names.pci_slot) < (int)sizeof(str)) - udev_builtin_add_property(dev, test, "ID_NET_NAME_SLOT", str); + if (names.pci_slot[0] && + snprintf_ok(str, sizeof str, "%s%s", prefix, names.pci_slot)) + udev_builtin_add_property(dev, test, "ID_NET_NAME_SLOT", str); goto out; } @@ -731,13 +727,13 @@ static int builtin_net_id(struct udev_device *dev, int argc, char *argv[], bool if (err >= 0 && names.type == NET_USB) { char str[IFNAMSIZ]; - if (names.pci_path[0]) - if (snprintf(str, sizeof(str), "%s%s%s", prefix, names.pci_path, names.usb_ports) < (int)sizeof(str)) - udev_builtin_add_property(dev, test, "ID_NET_NAME_PATH", str); + if (names.pci_path[0] && + snprintf_ok(str, sizeof str, "%s%s%s", prefix, names.pci_path, names.usb_ports)) + udev_builtin_add_property(dev, test, "ID_NET_NAME_PATH", str); - if (names.pci_slot[0]) - if (snprintf(str, sizeof(str), "%s%s%s", prefix, names.pci_slot, names.usb_ports) < (int)sizeof(str)) - udev_builtin_add_property(dev, test, "ID_NET_NAME_SLOT", str); + if (names.pci_slot[0] && + snprintf_ok(str, sizeof str, "%s%s%s", prefix, names.pci_slot, names.usb_ports)) + udev_builtin_add_property(dev, test, "ID_NET_NAME_SLOT", str); goto out; } @@ -746,13 +742,13 @@ static int builtin_net_id(struct udev_device *dev, int argc, char *argv[], bool if (err >= 0 && names.type == NET_BCMA) { char str[IFNAMSIZ]; - if (names.pci_path[0]) - if (snprintf(str, sizeof(str), "%s%s%s", prefix, names.pci_path, names.bcma_core) < (int)sizeof(str)) - udev_builtin_add_property(dev, test, "ID_NET_NAME_PATH", str); + if (names.pci_path[0] && + snprintf_ok(str, sizeof str, "%s%s%s", prefix, names.pci_path, names.bcma_core)) + udev_builtin_add_property(dev, test, "ID_NET_NAME_PATH", str); - if (names.pci_slot[0]) - if (snprintf(str, sizeof(str), "%s%s%s", prefix, names.pci_slot, names.bcma_core) < (int)sizeof(str)) - udev_builtin_add_property(dev, test, "ID_NET_NAME_SLOT", str); + if (names.pci_slot[0] && + snprintf(str, sizeof str, "%s%s%s", prefix, names.pci_slot, names.bcma_core)) + udev_builtin_add_property(dev, test, "ID_NET_NAME_SLOT", str); goto out; } out: