Compare commits

...

8 Commits

Author SHA1 Message Date
Yu Watanabe e16f18bddf
Merge pull request #14339 from keszybz/invalid-enablement-logs
Small enhancements to logs for enabling of invalid units
2019-12-17 15:04:14 +09:00
Yu Watanabe a0f11d1d11 random-util: call initialize_srand() after fork() 2019-12-17 15:03:36 +09:00
Anita Zhang 024941a521
Merge pull request #14351 from yuwata/util-constify-strv-xxx
util: constify arguments of strv_xxx()
2019-12-16 18:08:04 -08:00
Zbigniew Jędrzejewski-Szmek b742942edf TODO: drop entry
Implemented in 7d1e91d1a9.
2019-12-16 14:19:49 +01:00
Zbigniew Jędrzejewski-Szmek e51712963b shared/install: log syntax error for invalid DefaultInstance=
Ideally, we would want to report this over back over dbus. But that is pretty hard,
because the unitfile parsing logic doesn't provide any feedback.
systemd-analyze verify also doesn't notice the issue, because it doesn't look
at the [Install] section at all. Let's print a message in the logs at least.
2019-12-16 14:19:49 +01:00
Yu Watanabe 479ddcdf5a util: constify arguments of strv_xxx() 2019-12-16 15:51:04 +09:00
Zbigniew Jędrzejewski-Szmek d7ceaf7261 shared/install: provide a nicer error message for invalid WantedBy=/Required= values
$ build/systemctl --user cat badinstall
 # /home/zbyszek/.config/systemd/user/badinstall.service
[Service]
ExecStart=true

[Install]
WantedBy=asdf

$ build/systemctl --user enable badinstall
Failed to enable unit: "asdf" is not a valid unit name.

Fixes #4209.
2019-12-13 19:30:36 +01:00
Zbigniew Jędrzejewski-Szmek d9c1c43e67 shared/install: remove duplicated check
install_info_add() does the exact same check.
2019-12-13 19:30:36 +01:00
6 changed files with 70 additions and 48 deletions

3
TODO
View File

@ -1033,12 +1033,11 @@ Features:
- allow Type=simple with PIDFile= - allow Type=simple with PIDFile=
https://bugzilla.redhat.com/show_bug.cgi?id=723942 https://bugzilla.redhat.com/show_bug.cgi?id=723942
- allow writing multiple conditions in unit files on one line - allow writing multiple conditions in unit files on one line
- load-fragment: when loading a unit file via a chain of symlinks
verify that it is not masked via any of the names traversed.
- introduce Type=pid-file - introduce Type=pid-file
- introduce mix of BindTo and Requisite - introduce mix of BindTo and Requisite
- add a concept of RemainAfterExit= to scope units - add a concept of RemainAfterExit= to scope units
- Allow multiple ExecStart= for all Type= settings, so that we can cover rescue.service nicely - Allow multiple ExecStart= for all Type= settings, so that we can cover rescue.service nicely
- add verification of [Install] section to systemd-analyze verify
* udev-link-config: * udev-link-config:
- Make sure ID_PATH is always exported and complete for - Make sure ID_PATH is always exported and complete for

View File

@ -7,6 +7,7 @@
#include <elf.h> #include <elf.h>
#include <errno.h> #include <errno.h>
#include <fcntl.h> #include <fcntl.h>
#include <pthread.h>
#include <stdbool.h> #include <stdbool.h>
#include <stdint.h> #include <stdint.h>
#include <stdlib.h> #include <stdlib.h>
@ -28,6 +29,8 @@
#include "siphash24.h" #include "siphash24.h"
#include "time-util.h" #include "time-util.h"
static bool srand_called = false;
int rdrand(unsigned long *ret) { int rdrand(unsigned long *ret) {
/* So, you are a "security researcher", and you wonder why we bother with using raw RDRAND here, /* So, you are a "security researcher", and you wonder why we bother with using raw RDRAND here,
@ -272,8 +275,12 @@ int genuine_random_bytes(void *p, size_t n, RandomFlags flags) {
return loop_read_exact(fd, p, n, true); return loop_read_exact(fd, p, n, true);
} }
static void clear_srand_initialization(void) {
srand_called = false;
}
void initialize_srand(void) { void initialize_srand(void) {
static bool srand_called = false; static bool pthread_atfork_registered = false;
unsigned x; unsigned x;
#if HAVE_SYS_AUXV_H #if HAVE_SYS_AUXV_H
const void *auxv; const void *auxv;
@ -309,6 +316,11 @@ void initialize_srand(void) {
srand(x); srand(x);
srand_called = true; srand_called = true;
if (!pthread_atfork_registered) {
(void) pthread_atfork(NULL, NULL, clear_srand_initialization);
pthread_atfork_registered = true;
}
} }
/* INT_MAX gives us only 31 bits, so use 24 out of that. */ /* INT_MAX gives us only 31 bits, so use 24 out of that. */

View File

@ -16,8 +16,8 @@
#include "string-util.h" #include "string-util.h"
#include "strv.h" #include "strv.h"
char *strv_find(char **l, const char *name) { char *strv_find(char * const *l, const char *name) {
char **i; char * const *i;
assert(name); assert(name);
@ -28,8 +28,8 @@ char *strv_find(char **l, const char *name) {
return NULL; return NULL;
} }
char *strv_find_prefix(char **l, const char *name) { char *strv_find_prefix(char * const *l, const char *name) {
char **i; char * const *i;
assert(name); assert(name);
@ -40,8 +40,8 @@ char *strv_find_prefix(char **l, const char *name) {
return NULL; return NULL;
} }
char *strv_find_startswith(char **l, const char *name) { char *strv_find_startswith(char * const *l, const char *name) {
char **i, *e; char * const *i, *e;
assert(name); assert(name);
@ -181,8 +181,8 @@ char **strv_new_internal(const char *x, ...) {
return r; return r;
} }
int strv_extend_strv(char ***a, char **b, bool filter_duplicates) { int strv_extend_strv(char ***a, char * const *b, bool filter_duplicates) {
char **s, **t; char * const *s, **t;
size_t p, q, i = 0, j; size_t p, q, i = 0, j;
assert(a); assert(a);
@ -228,9 +228,9 @@ rollback:
return -ENOMEM; return -ENOMEM;
} }
int strv_extend_strv_concat(char ***a, char **b, const char *suffix) { int strv_extend_strv_concat(char ***a, char * const *b, const char *suffix) {
char * const *s;
int r; int r;
char **s;
STRV_FOREACH(s, b) { STRV_FOREACH(s, b) {
char *v; char *v;
@ -346,9 +346,9 @@ int strv_split_extract(char ***t, const char *s, const char *separators, Extract
return (int) n; return (int) n;
} }
char *strv_join_prefix(char **l, const char *separator, const char *prefix) { char *strv_join_prefix(char * const *l, const char *separator, const char *prefix) {
char * const *s;
char *r, *e; char *r, *e;
char **s;
size_t n, k, m; size_t n, k, m;
if (!separator) if (!separator)
@ -562,8 +562,8 @@ char **strv_uniq(char **l) {
return l; return l;
} }
bool strv_is_uniq(char **l) { bool strv_is_uniq(char * const *l) {
char **i; char * const *i;
STRV_FOREACH(i, l) STRV_FOREACH(i, l)
if (strv_find(i+1, *i)) if (strv_find(i+1, *i))
@ -666,7 +666,7 @@ char **strv_split_nulstr(const char *s) {
return r; return r;
} }
int strv_make_nulstr(char **l, char **p, size_t *q) { int strv_make_nulstr(char * const *l, char **ret, size_t *ret_size) {
/* A valid nulstr with two NULs at the end will be created, but /* A valid nulstr with two NULs at the end will be created, but
* q will be the length without the two trailing NULs. Thus the output * q will be the length without the two trailing NULs. Thus the output
* string is a valid nulstr and can be iterated over using NULSTR_FOREACH, * string is a valid nulstr and can be iterated over using NULSTR_FOREACH,
@ -676,10 +676,10 @@ int strv_make_nulstr(char **l, char **p, size_t *q) {
size_t n_allocated = 0, n = 0; size_t n_allocated = 0, n = 0;
_cleanup_free_ char *m = NULL; _cleanup_free_ char *m = NULL;
char **i; char * const *i;
assert(p); assert(ret);
assert(q); assert(ret_size);
STRV_FOREACH(i, l) { STRV_FOREACH(i, l) {
size_t z; size_t z;
@ -703,16 +703,16 @@ int strv_make_nulstr(char **l, char **p, size_t *q) {
m[n] = '\0'; m[n] = '\0';
assert(n > 0); assert(n > 0);
*p = m; *ret = m;
*q = n - 1; *ret_size = n - 1;
m = NULL; m = NULL;
return 0; return 0;
} }
bool strv_overlap(char **a, char **b) { bool strv_overlap(char * const *a, char * const *b) {
char **i; char * const *i;
STRV_FOREACH(i, a) STRV_FOREACH(i, a)
if (strv_contains(b, *i)) if (strv_contains(b, *i))
@ -730,7 +730,7 @@ char **strv_sort(char **l) {
return l; return l;
} }
bool strv_equal(char **a, char **b) { bool strv_equal(char * const *a, char * const *b) {
if (strv_isempty(a)) if (strv_isempty(a))
return strv_isempty(b); return strv_isempty(b);
@ -745,8 +745,8 @@ bool strv_equal(char **a, char **b) {
return true; return true;
} }
void strv_print(char **l) { void strv_print(char * const *l) {
char **s; char * const *s;
STRV_FOREACH(s, l) STRV_FOREACH(s, l)
puts(*s); puts(*s);
@ -874,9 +874,9 @@ rollback:
return -ENOMEM; return -ENOMEM;
} }
int fputstrv(FILE *f, char **l, const char *separator, bool *space) { int fputstrv(FILE *f, char * const *l, const char *separator, bool *space) {
bool b = false; bool b = false;
char **s; char * const *s;
int r; int r;
/* Like fputs(), but for strv, and with a less stupid argument order */ /* Like fputs(), but for strv, and with a less stupid argument order */

View File

@ -13,9 +13,9 @@
#include "macro.h" #include "macro.h"
#include "string-util.h" #include "string-util.h"
char *strv_find(char **l, const char *name) _pure_; char *strv_find(char * const *l, const char *name) _pure_;
char *strv_find_prefix(char **l, const char *name) _pure_; char *strv_find_prefix(char * const *l, const char *name) _pure_;
char *strv_find_startswith(char **l, const char *name) _pure_; char *strv_find_startswith(char * const *l, const char *name) _pure_;
char **strv_free(char **l); char **strv_free(char **l);
DEFINE_TRIVIAL_CLEANUP_FUNC(char**, strv_free); DEFINE_TRIVIAL_CLEANUP_FUNC(char**, strv_free);
@ -30,8 +30,8 @@ void strv_clear(char **l);
char **strv_copy(char * const *l); char **strv_copy(char * const *l);
size_t strv_length(char * const *l) _pure_; size_t strv_length(char * const *l) _pure_;
int strv_extend_strv(char ***a, char **b, bool filter_duplicates); int strv_extend_strv(char ***a, char * const *b, bool filter_duplicates);
int strv_extend_strv_concat(char ***a, char **b, const char *suffix); int strv_extend_strv_concat(char ***a, char * const *b, const char *suffix);
int strv_extend(char ***l, const char *value); int strv_extend(char ***l, const char *value);
int strv_extendf(char ***l, const char *format, ...) _printf_(2,0); int strv_extendf(char ***l, const char *format, ...) _printf_(2,0);
int strv_extend_front(char ***l, const char *value); int strv_extend_front(char ***l, const char *value);
@ -49,9 +49,9 @@ int strv_consume_prepend(char ***l, char *value);
char **strv_remove(char **l, const char *s); char **strv_remove(char **l, const char *s);
char **strv_uniq(char **l); char **strv_uniq(char **l);
bool strv_is_uniq(char **l); bool strv_is_uniq(char * const *l);
bool strv_equal(char **a, char **b); bool strv_equal(char * const *a, char * const *b);
#define strv_contains(l, s) (!!strv_find((l), (s))) #define strv_contains(l, s) (!!strv_find((l), (s)))
@ -77,16 +77,16 @@ char **strv_split_newlines(const char *s);
int strv_split_extract(char ***t, const char *s, const char *separators, ExtractFlags flags); int strv_split_extract(char ***t, const char *s, const char *separators, ExtractFlags flags);
char *strv_join_prefix(char **l, const char *separator, const char *prefix); char *strv_join_prefix(char * const *l, const char *separator, const char *prefix);
static inline char *strv_join(char **l, const char *separator) { static inline char *strv_join(char * const *l, const char *separator) {
return strv_join_prefix(l, separator, NULL); return strv_join_prefix(l, separator, NULL);
} }
char **strv_parse_nulstr(const char *s, size_t l); char **strv_parse_nulstr(const char *s, size_t l);
char **strv_split_nulstr(const char *s); char **strv_split_nulstr(const char *s);
int strv_make_nulstr(char **l, char **p, size_t *n); int strv_make_nulstr(char * const *l, char **p, size_t *n);
bool strv_overlap(char **a, char **b) _pure_; bool strv_overlap(char * const *a, char * const *b) _pure_;
#define STRV_FOREACH(s, l) \ #define STRV_FOREACH(s, l) \
for ((s) = (l); (s) && *(s); (s)++) for ((s) = (l); (s) && *(s); (s)++)
@ -103,7 +103,7 @@ bool strv_overlap(char **a, char **b) _pure_;
for ((x) = (l), (y) = (x+1); (x) && *(x) && *(y); (x) += 2, (y) = (x + 1)) for ((x) = (l), (y) = (x+1); (x) && *(x) && *(y); (x) += 2, (y) = (x + 1))
char **strv_sort(char **l); char **strv_sort(char **l);
void strv_print(char **l); void strv_print(char * const *l);
#define STRV_MAKE(...) ((char**) ((const char*[]) { __VA_ARGS__, NULL })) #define STRV_MAKE(...) ((char**) ((const char*[]) { __VA_ARGS__, NULL }))
@ -192,7 +192,7 @@ char **strv_skip(char **l, size_t n);
int strv_extend_n(char ***l, const char *value, size_t n); int strv_extend_n(char ***l, const char *value, size_t n);
int fputstrv(FILE *f, char **l, const char *separator, bool *space); int fputstrv(FILE *f, char * const *l, const char *separator, bool *space);
#define strv_free_and_replace(a, b) \ #define strv_free_and_replace(a, b) \
({ \ ({ \

View File

@ -1902,6 +1902,12 @@ static int install_error(
"Unit %s is transient or generated.", changes[i].path); "Unit %s is transient or generated.", changes[i].path);
goto found; goto found;
case -EUCLEAN:
r = sd_bus_error_setf(error, BUS_ERROR_BAD_UNIT_SETTING,
"\"%s\" is not a valid unit name.",
changes[i].path);
goto found;
case -ELOOP: case -ELOOP:
r = sd_bus_error_setf(error, BUS_ERROR_UNIT_LINKED, r = sd_bus_error_setf(error, BUS_ERROR_UNIT_LINKED,
"Refusing to operate on alias name or linked unit file: %s", "Refusing to operate on alias name or linked unit file: %s",

View File

@ -362,6 +362,12 @@ void unit_file_dump_changes(int r, const char *verb, const UnitFileChange *chang
verb, changes[i].path); verb, changes[i].path);
logged = true; logged = true;
break; break;
case -EUCLEAN:
log_error_errno(changes[i].type,
"Failed to %s unit, \"%s\" is not a valid unit name.",
verb, changes[i].path);
logged = true;
break;
case -ELOOP: case -ELOOP:
log_error_errno(changes[i].type, "Failed to %s unit, refusing to operate on linked unit file %s", log_error_errno(changes[i].type, "Failed to %s unit, refusing to operate on linked unit file %s",
verb, changes[i].path); verb, changes[i].path);
@ -1138,9 +1144,6 @@ static int config_parse_also(
if (r < 0) if (r < 0)
return r; return r;
if (!unit_name_is_valid(printed, UNIT_NAME_ANY))
return -EINVAL;
r = install_info_add(c, printed, NULL, true, &alsoinfo); r = install_info_add(c, printed, NULL, true, &alsoinfo);
if (r < 0) if (r < 0)
return r; return r;
@ -1194,7 +1197,8 @@ static int config_parse_default_instance(
} }
if (!unit_instance_is_valid(printed)) if (!unit_instance_is_valid(printed))
return -EINVAL; return log_syntax(unit, LOG_WARNING, filename, line, SYNTHETIC_ERRNO(EINVAL),
"Invalid DefaultInstance= value \"%s\".", printed);
return free_and_replace(i->default_instance, printed); return free_and_replace(i->default_instance, printed);
} }
@ -1797,7 +1801,8 @@ static int install_info_symlink_wants(
return q; return q;
if (!unit_name_is_valid(dst, UNIT_NAME_ANY)) { if (!unit_name_is_valid(dst, UNIT_NAME_ANY)) {
r = -EINVAL; unit_file_changes_add(changes, n_changes, -EUCLEAN, dst, NULL);
r = -EUCLEAN;
continue; continue;
} }