1
0
mirror of https://github.com/systemd/systemd synced 2026-03-17 10:34:46 +01:00

Compare commits

..

11 Commits

Author SHA1 Message Date
Yu Watanabe
a8f76a8db7
Merge pull request #19766 from keszybz/fuzz-fixes
Fuzz fixes
2021-06-01 11:14:45 +09:00
Lennart Poettering
6f2ef23a4e udevadm: output trigger UUID in UUID format, instead of ID128
The SYNTH_UUID property also shows it in UUID format, and so does the
kernel and its docs otherwise, hence accept our fate and also output it
in UUID.
2021-06-01 11:14:03 +09:00
Luca Boccassi
6c498f6d89
Merge pull request #19765 from keszybz/early-boot-logging-improvements
Early boot logging improvements
2021-05-31 22:59:51 +01:00
Zbigniew Jędrzejewski-Szmek
2dd7a72d5a fuzz-journal-remote: print some kinds of errors
In https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34803, we fail with:

  Assertion 'IN_SET(r, -ENOMEM, -EMFILE, -ENFILE)' failed at src/journal-remote/fuzz-journal-remote.c:69,
    function int LLVMFuzzerTestOneInput(const uint8_t *, size_t)(). Aborting.
  AddressSanitizer:DEADLYSIGNAL

Let's try to print the error, so maybe we can see what is going on.
With the previous commit we shouldn't print out anything.
2021-05-31 19:29:07 +02:00
Zbigniew Jędrzejewski-Szmek
23b8aa648d journal-remote: downgrade messages about input data to warnings
Those are unexpected, so a user-visible message seems appropriate.
But they are not our errors, and to some extent we can recover from
them, so "warning" seems more appropriate than "error".
2021-05-31 19:29:07 +02:00
Zbigniew Jędrzejewski-Szmek
a88f9dbae2 systemctl: unset const char* arguments in static destructors
When fuzzing, the following happens:
- we parse 'data' and produce an argv array,
- one of the items in argv is assigned to arg_host,
- the argv array is subsequently freed by strv_freep(), and arg_host has a dangling symlink.

In normal use, argv is static, so arg_host can never become a dangling pointer.
In fuzz-systemctl-parse-argv, if we repeatedly parse the same array, we
have some dangling pointers while we're in the middle of parsing. If we parse
the same array a second time, at the end all the dangling pointers will have been
replaced again. But for a short time, if parsing one of the arguments uses another
argument, we would use a dangling pointer.

Such a case occurs when we have --host=… --boot-loader-entry=help. The latter calls
acquire_bus() which uses arg_host.

I'm not particularly happy with making the code more complicated just for
fuzzing, but I think it's better to resolve this, even if the issue cannot
occur in normal invocations, than to deal with fuzzer reports.

Should fix https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=31714.
2021-05-31 19:29:07 +02:00
Zbigniew Jędrzejewski-Szmek
e7b18106f8 pid1: remove dot from initial announcement
This line is so long, that the end is usually not visible on
the terminal. The dot looks out of place, and dropping it saves one
column for more interesting content.
2021-05-31 19:22:51 +02:00
Zbigniew Jędrzejewski-Szmek
44a989e039 tty-ask-password-agent: log when starting a query on the console
When looking at logs from a boot with an encrypted device, I see
(with unrelevant messages snipped):
[    2.751692] systemd[1]: Started Dispatch Password Requests to Console.
[    7.929199] systemd-cryptsetup[258]: Set cipher aes, mode xts-plain64, key size 512 bits for device /dev/disk/by-uuid/2d9b648a-15b1-4204-988b-ec085089f8ce.
[    9.499483] systemd[1]: Finished Cryptography Setup for luks-2d9b648a-15b1-4204-988b-ec085089f8ce.

There is a hug gap in timing without any explanatory message. If I didn't type
in the password, there would be no way to figure out why things blocked from
this log, so let's log something to the log too.
2021-05-31 19:22:51 +02:00
Zbigniew Jędrzejewski-Szmek
0727077b53 tty-ask-password-agent: highlight summary in help 2021-05-31 19:22:51 +02:00
Zbigniew Jędrzejewski-Szmek
af88c399c0 tty-ask-password-agent: mention optional argument in help
0cf84693877f060254f04cf38120f52c2aa3059c added --console.
6af621248f2255f9ce50b0bafdde475305dc4e57 added an optional argument, but didn't
update the help texts.

Note that there is no ambiguity with the optional argument because no positional
arguments are allowed.
2021-05-31 19:22:51 +02:00
Zbigniew Jędrzejewski-Szmek
6b42227edb systemctl: put static destructor in the order of variables 2021-05-31 12:17:19 +02:00
13 changed files with 61 additions and 38 deletions

View File

@ -92,11 +92,11 @@
</varlistentry> </varlistentry>
<varlistentry> <varlistentry>
<term><option>--console</option></term> <term><option>--console</option><optional>=<replaceable>DEVICE</replaceable></optional></term>
<listitem><para>Ask question on <listitem><para>Ask question on TTY <replaceable>DEVICE</replaceable> instead of querying the user on
<filename>/dev/console</filename> instead of querying the user the calling TTY. If <replaceable>DEVICE</replaceable> is not specified,
on the calling TTY. </para></listitem> <filename>/dev/console</filename> will be used.</para></listitem>
</varlistentry> </varlistentry>
<xi:include href="standard-options.xml" xpointer="help" /> <xi:include href="standard-options.xml" xpointer="help" />

View File

@ -79,6 +79,13 @@ void* memdup_suffix0(const void *p, size_t l); /* We can't use _alloc_() here, s
memcpy_safe(_q_, p, _l_); \ memcpy_safe(_q_, p, _l_); \
}) })
static inline void unsetp(void *p) {
/* A trivial "destructor" that can be used in cases where we want to
* unset a pointer from a _cleanup_ function. */
*(void**)p = NULL;
}
static inline void freep(void *p) { static inline void freep(void *p) {
*(void**)p = mfree(*(void**) p); *(void**)p = mfree(*(void**) p);
} }

View File

@ -1999,7 +1999,7 @@ static void log_execution_mode(bool *ret_first_boot) {
if (arg_system) { if (arg_system) {
int v; int v;
log_info("systemd " GIT_VERSION " running in %ssystem mode. (%s)", log_info("systemd " GIT_VERSION " running in %ssystem mode (%s)",
arg_action == ACTION_TEST ? "test " : "", arg_action == ACTION_TEST ? "test " : "",
systemd_features); systemd_features);

View File

@ -28,7 +28,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
return 0; return 0;
if (!getenv("SYSTEMD_LOG_LEVEL")) if (!getenv("SYSTEMD_LOG_LEVEL"))
log_set_max_level(LOG_CRIT); log_set_max_level(LOG_ERR);
fdin = memfd_new_and_map("fuzz-journal-remote", size, &mem); fdin = memfd_new_and_map("fuzz-journal-remote", size, &mem);
if (fdin < 0) if (fdin < 0)
@ -66,6 +66,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
r = sd_journal_open_files(&j, (const char**) STRV_MAKE(name), 0); r = sd_journal_open_files(&j, (const char**) STRV_MAKE(name), 0);
if (r < 0) { if (r < 0) {
log_error_errno(r, "sd_journal_open_files([\"%s\"]) failed: %m", name);
assert_se(IN_SET(r, -ENOMEM, -EMFILE, -ENFILE)); assert_se(IN_SET(r, -ENOMEM, -EMFILE, -ENFILE));
return r; return r;
} }

View File

@ -74,7 +74,7 @@ int process_source(RemoteSource *source, bool compress, bool seal) {
&source->importer.boot_id, &source->importer.boot_id,
compress, seal); compress, seal);
if (r == -EBADMSG) { if (r == -EBADMSG) {
log_error_errno(r, "Entry is invalid, ignoring."); log_warning_errno(r, "Entry is invalid, ignoring.");
r = 0; r = 0;
} else if (r < 0) } else if (r < 0)
log_error_errno(r, "Failed to write entry of %zu bytes: %m", log_error_errno(r, "Failed to write entry of %zu bytes: %m",

View File

@ -69,7 +69,7 @@ static int get_line(JournalImporter *imp, char **line, size_t *size) {
imp->scanned = imp->filled; imp->scanned = imp->filled;
if (imp->scanned >= DATA_SIZE_MAX) if (imp->scanned >= DATA_SIZE_MAX)
return log_error_errno(SYNTHETIC_ERRNO(ENOBUFS), return log_warning_errno(SYNTHETIC_ERRNO(ENOBUFS),
"Entry is bigger than %u bytes.", "Entry is bigger than %u bytes.",
DATA_SIZE_MAX); DATA_SIZE_MAX);
@ -163,7 +163,7 @@ static int get_data_size(JournalImporter *imp) {
imp->data_size = unaligned_read_le64(data); imp->data_size = unaligned_read_le64(data);
if (imp->data_size > DATA_SIZE_MAX) if (imp->data_size > DATA_SIZE_MAX)
return log_error_errno(SYNTHETIC_ERRNO(EINVAL), return log_warning_errno(SYNTHETIC_ERRNO(EINVAL),
"Stream declares field with size %zu > DATA_SIZE_MAX = %u", "Stream declares field with size %zu > DATA_SIZE_MAX = %u",
imp->data_size, DATA_SIZE_MAX); imp->data_size, DATA_SIZE_MAX);
if (imp->data_size == 0) if (imp->data_size == 0)
@ -203,7 +203,7 @@ static int get_data_newline(JournalImporter *imp) {
int l; int l;
l = cescape_char(*data, buf); l = cescape_char(*data, buf);
return log_error_errno(SYNTHETIC_ERRNO(EINVAL), return log_warning_errno(SYNTHETIC_ERRNO(EINVAL),
"Expected newline, got '%.*s'", l, buf); "Expected newline, got '%.*s'", l, buf);
} }

View File

@ -22,7 +22,7 @@ int kill_unit(int argc, char *argv[], void *userdata) {
arg_kill_who = "all"; arg_kill_who = "all";
/* --fail was specified */ /* --fail was specified */
if (streq(arg_job_mode, "fail")) if (streq(arg_job_mode(), "fail"))
kill_who = strjoina(arg_kill_who, "-fail"); kill_who = strjoina(arg_kill_who, "-fail");
r = expand_unit_names(bus, strv_skip(argv, 1), NULL, &names, NULL); r = expand_unit_names(bus, strv_skip(argv, 1), NULL, &names, NULL);

View File

@ -306,7 +306,7 @@ int start_unit(int argc, char *argv[], void *userdata) {
/* A command in style of "systemctl start <unit1> <unit2> …", "sysemctl stop <unit1> <unit2> …" and so on */ /* A command in style of "systemctl start <unit1> <unit2> …", "sysemctl stop <unit1> <unit2> …" and so on */
method = verb_to_method(argv[0]); method = verb_to_method(argv[0]);
job_type = verb_to_job_type(argv[0]); job_type = verb_to_job_type(argv[0]);
mode = arg_job_mode; mode = arg_job_mode();
} else } else
method = job_type = mode = NULL; method = job_type = mode = NULL;

View File

@ -65,7 +65,7 @@ char **arg_states = NULL;
char **arg_properties = NULL; char **arg_properties = NULL;
bool arg_all = false; bool arg_all = false;
enum dependency arg_dependency = DEPENDENCY_FORWARD; enum dependency arg_dependency = DEPENDENCY_FORWARD;
const char *arg_job_mode = "replace"; const char *_arg_job_mode = NULL;
UnitFileScope arg_scope = UNIT_FILE_SYSTEM; UnitFileScope arg_scope = UNIT_FILE_SYSTEM;
bool arg_wait = false; bool arg_wait = false;
bool arg_no_block = false; bool arg_no_block = false;
@ -112,11 +112,16 @@ bool arg_read_only = false;
bool arg_mkdir = false; bool arg_mkdir = false;
bool arg_marked = false; bool arg_marked = false;
STATIC_DESTRUCTOR_REGISTER(arg_wall, strv_freep);
STATIC_DESTRUCTOR_REGISTER(arg_root, freep);
STATIC_DESTRUCTOR_REGISTER(arg_types, strv_freep); STATIC_DESTRUCTOR_REGISTER(arg_types, strv_freep);
STATIC_DESTRUCTOR_REGISTER(arg_states, strv_freep); STATIC_DESTRUCTOR_REGISTER(arg_states, strv_freep);
STATIC_DESTRUCTOR_REGISTER(arg_properties, strv_freep); STATIC_DESTRUCTOR_REGISTER(arg_properties, strv_freep);
STATIC_DESTRUCTOR_REGISTER(_arg_job_mode, unsetp);
STATIC_DESTRUCTOR_REGISTER(arg_wall, strv_freep);
STATIC_DESTRUCTOR_REGISTER(arg_kill_who, unsetp);
STATIC_DESTRUCTOR_REGISTER(arg_root, freep);
STATIC_DESTRUCTOR_REGISTER(arg_reboot_argument, unsetp);
STATIC_DESTRUCTOR_REGISTER(arg_host, unsetp);
STATIC_DESTRUCTOR_REGISTER(arg_boot_loader_entry, unsetp);
STATIC_DESTRUCTOR_REGISTER(arg_clean_what, strv_freep); STATIC_DESTRUCTOR_REGISTER(arg_clean_what, strv_freep);
static int systemctl_help(void) { static int systemctl_help(void) {
@ -598,19 +603,19 @@ static int systemctl_parse_argv(int argc, char *argv[]) {
break; break;
case ARG_JOB_MODE: case ARG_JOB_MODE:
arg_job_mode = optarg; _arg_job_mode = optarg;
break; break;
case ARG_FAIL: case ARG_FAIL:
arg_job_mode = "fail"; _arg_job_mode = "fail";
break; break;
case ARG_IRREVERSIBLE: case ARG_IRREVERSIBLE:
arg_job_mode = "replace-irreversibly"; _arg_job_mode = "replace-irreversibly";
break; break;
case ARG_IGNORE_DEPENDENCIES: case ARG_IGNORE_DEPENDENCIES:
arg_job_mode = "ignore-dependencies"; _arg_job_mode = "ignore-dependencies";
break; break;
case ARG_USER: case ARG_USER:

View File

@ -49,7 +49,7 @@ extern char **arg_states;
extern char **arg_properties; extern char **arg_properties;
extern bool arg_all; extern bool arg_all;
extern enum dependency arg_dependency; extern enum dependency arg_dependency;
extern const char *arg_job_mode; extern const char *_arg_job_mode;
extern UnitFileScope arg_scope; extern UnitFileScope arg_scope;
extern bool arg_wait; extern bool arg_wait;
extern bool arg_no_block; extern bool arg_no_block;
@ -96,4 +96,8 @@ extern bool arg_read_only;
extern bool arg_mkdir; extern bool arg_mkdir;
extern bool arg_marked; extern bool arg_marked;
static inline const char* arg_job_mode(void) {
return _arg_job_mode ?: "replace";
}
int systemctl_dispatch_parse_argv(int argc, char *argv[]); int systemctl_dispatch_parse_argv(int argc, char *argv[]);

View File

@ -143,10 +143,9 @@ static int agent_ask_password_tty(
char ***ret) { char ***ret) {
int tty_fd = -1, r; int tty_fd = -1, r;
if (arg_console) {
const char *con = arg_device ?: "/dev/console"; const char *con = arg_device ?: "/dev/console";
if (arg_console) {
tty_fd = acquire_terminal(con, ACQUIRE_TERMINAL_WAIT, USEC_INFINITY); tty_fd = acquire_terminal(con, ACQUIRE_TERMINAL_WAIT, USEC_INFINITY);
if (tty_fd < 0) if (tty_fd < 0)
return log_error_errno(tty_fd, "Failed to acquire %s: %m", con); return log_error_errno(tty_fd, "Failed to acquire %s: %m", con);
@ -155,6 +154,7 @@ static int agent_ask_password_tty(
if (r < 0) if (r < 0)
log_warning_errno(r, "Failed to reset terminal, ignoring: %m"); log_warning_errno(r, "Failed to reset terminal, ignoring: %m");
log_info("Starting password query on %s.", con);
} }
r = ask_password_tty(tty_fd, message, NULL, until, flags, flag_file, ret); r = ask_password_tty(tty_fd, message, NULL, until, flags, flag_file, ret);
@ -162,6 +162,9 @@ static int agent_ask_password_tty(
if (arg_console) { if (arg_console) {
tty_fd = safe_close(tty_fd); tty_fd = safe_close(tty_fd);
release_terminal(); release_terminal();
if (r >= 0)
log_info("Password query on %s finished successfully.", con);
} }
return r; return r;
@ -411,7 +414,7 @@ static int help(void) {
return log_oom(); return log_oom();
printf("%s [OPTIONS...]\n\n" printf("%s [OPTIONS...]\n\n"
"Process system password requests.\n\n" "%sProcess system password requests.%s\n\n"
" -h --help Show this help\n" " -h --help Show this help\n"
" --version Show package version\n" " --version Show package version\n"
" --list Show pending password requests\n" " --list Show pending password requests\n"
@ -419,9 +422,12 @@ static int help(void) {
" --watch Continuously process password requests\n" " --watch Continuously process password requests\n"
" --wall Continuously forward password requests to wall\n" " --wall Continuously forward password requests to wall\n"
" --plymouth Ask question with Plymouth instead of on TTY\n" " --plymouth Ask question with Plymouth instead of on TTY\n"
" --console Ask question on /dev/console instead of current TTY\n" " --console[=DEVICE] Ask question on /dev/console (or DEVICE if specified)\n"
" instead of the current TTY\n"
"\nSee the %s for details.\n", "\nSee the %s for details.\n",
program_invocation_short_name, program_invocation_short_name,
ansi_highlight(),
ansi_normal(),
link); link);
return 0; return 0;

View File

@ -109,7 +109,7 @@ static int exec_list(
/* If the user asked for it, write event UUID to stdout */ /* If the user asked for it, write event UUID to stdout */
if (arg_uuid) if (arg_uuid)
printf(SD_ID128_FORMAT_STR "\n", SD_ID128_FORMAT_VAL(id)); printf(SD_ID128_UUID_FORMAT_STR "\n", SD_ID128_FORMAT_VAL(id));
if (settle_hashmap) { if (settle_hashmap) {
_cleanup_free_ sd_id128_t *mid = NULL; _cleanup_free_ sd_id128_t *mid = NULL;
@ -175,7 +175,7 @@ static int device_monitor_handler(sd_device_monitor *m, sd_device *dev, void *us
printf("settle %s\n", syspath); printf("settle %s\n", syspath);
if (arg_uuid) if (arg_uuid)
printf("settle " SD_ID128_FORMAT_STR "\n", SD_ID128_FORMAT_VAL(*settle_id)); printf("settle " SD_ID128_UUID_FORMAT_STR "\n", SD_ID128_FORMAT_VAL(*settle_id));
free(hashmap_remove(settle_hashmap, syspath)); free(hashmap_remove(settle_hashmap, syspath));
free(k); free(k);

Binary file not shown.