dasharo/systemd
0
0
Fork 0
mirror of https://github.com/Dasharo/systemd.git synced 2026-03-06 15:02:31 -08:00
Code Issues Packages Projects Releases Wiki Activity

install: follow unit file symlinks in /usr, but not /etc when looking for [Install] data

Browse Source 
Some distributions use alias unit files via symlinks in /usr to cover
for legacy service names. With this change we'll allow "systemctl
enable" on such aliases.

Previously, our rule was that symlinks are user configuration that
"systemctl enable" + "systemctl disable" creates and removes, while unit
files is where the instructions to do so are store. As a result of the
rule we'd never read install information through symlinks, since that
would mix enablement state with installation instructions.

Now, the new rule is that only symlinks inside of /etc are
configuration. Unit files, and symlinks in /usr are now valid for
installation instructions.

This patch is quite a rework of the whole install logic, and makes the
following addional changes:

- Adds a complete test "test-instal-root" that tests the install logic
  pretty comprehensively.

- Never uses canonicalize_file_name(), because that's incompatible with
  operation relative to a specific root directory.

- unit_file_get_state() is reworked to return a proper error, and
  returns the state in a call-by-ref parameter. This cleans up confusion
  between the enum type and errno-like errors.

- The new logic puts a limit on how long to follow unit file symlinks:
  it will do so only for 64 steps at max.

- The InstallContext object's fields are renamed to will_process and
  has_processed (will_install and has_installed) since they are also
  used for deinstallation and all kinds of other operations.

- The root directory is always verified before use.

- install.c is reordered to place the exported functions together.

- Stricter rules are followed when traversing symlinks: the unit suffix
  must say identical, and it's not allowed to link between regular units
  and templated units.

- Various modernizations

- The "invalid" unit file state has been renamed to "bad", in order to
  avoid confusion between UNIT_FILE_INVALID and
  _UNIT_FILE_STATE_INVALID. Given that the state should normally not be
  seen and is not documented this should not be a problematic change.
  The new name is now documented however.

Fixes #1375, #1718, #1706
This commit is contained in:
Lennart Poettering
2015-10-08 22:31:56 +02:00
parent d073dea0a8
commit 0ec0deaa30
12 changed files with 2106 additions and 1074 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
Makefile.am
View File

@@ -1493,7 +1493,8 @@ tests += \
test-verbs \
test-af-list \
test-arphrd-list \
test-dns-domain
test-dns-domain \
test-install-root
EXTRA_DIST += \
test/a.service \
@@ -1836,6 +1837,12 @@ test_verbs_SOURCES = \
test_verbs_LDADD = \
libshared.la
test_install_root_SOURCES = \
src/test/test-install-root.c
test_install_root_LDADD = \
libshared.la
test_namespace_LDADD = \
libcore.la

14
man/systemctl.xml
View File

@@ -961,10 +961,11 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service
<term><command>list-unit-files <optional><replaceable>PATTERN...</replaceable></optional></command></term>
<listitem>
<para>List installed unit files. If one or more
<replaceable>PATTERN</replaceable>s are specified, only
units whose filename (just the last component of the path)
matches one of them are shown.</para>
<para>List installed unit files and their enablement state
(as reported by <command>is-enabled</command>). If one or
more <replaceable>PATTERN</replaceable>s are specified,
only units whose filename (just the last component of the
path) matches one of them are shown.</para>
</listitem>
</varlistentry>
@@ -1171,6 +1172,11 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service
<entry>The unit file is not enabled.</entry>
<entry>&gt; 0</entry>
</row>
<row>
<entry><literal>bad</literal></entry>
<entry>Unit file is invalid or another error occured. Note that <command>is-enabled</command> will not actually return this state, but print an error message instead. However the unit file listing printed by <command>list-unit-files</command> might show it.</entry>
<entry>&gt; 0</entry>
</row>
</tbody>
</tgroup>
</table>

20
src/basic/fs-util.c
View File

@@ -233,6 +233,26 @@ int readlink_and_canonicalize(const char *p, char **r) {
return 0;
}
int readlink_and_make_absolute_root(const char *root, const char *path, char **ret) {
_cleanup_free_ char *target = NULL, *t = NULL;
const char *full;
int r;
full = prefix_roota(root, path);
r = readlink_malloc(full, &target);
if (r < 0)
return r;
t = file_in_same_dir(path, target);
if (!t)
return -ENOMEM;
*ret = t;
t = NULL;
return 0;
}
int chmod_and_chown(const char *path, mode_t mode, uid_t uid, gid_t gid) {
assert(path);

1
src/basic/fs-util.h
View File

@@ -40,6 +40,7 @@ int readlink_malloc(const char *p, char **r);
int readlink_value(const char *p, char **ret);
int readlink_and_make_absolute(const char *p, char **r);
int readlink_and_canonicalize(const char *p, char **r);
int readlink_and_make_absolute_root(const char *root, const char *path, char **ret);
int chmod_and_chown(const char *path, mode_t mode, uid_t uid, gid_t gid);
int fchmod_and_fchown(int fd, mode_t mode, uid_t uid, gid_t gid);

6
src/core/dbus-manager.c
View File

@@ -1521,9 +1521,9 @@ static int method_get_unit_file_state(sd_bus_message *message, void *userdata, s
scope = m->running_as == MANAGER_SYSTEM ? UNIT_FILE_SYSTEM : UNIT_FILE_USER;
state = unit_file_get_state(scope, NULL, name);
if (state < 0)
return state;
r = unit_file_get_state(scope, NULL, name, &state);
if (r < 0)
return r;
return sd_bus_reply_method_return(message, "s", unit_file_state_to_string(state));
}

16
src/core/unit.c
View File

@@ -3148,12 +3148,19 @@ int unit_following_set(Unit *u, Set **s) {
}
UnitFileState unit_get_unit_file_state(Unit *u) {
int r;
assert(u);
if (u->unit_file_state < 0 && u->fragment_path)
u->unit_file_state = unit_file_get_state(
if (u->unit_file_state < 0 && u->fragment_path) {
r = unit_file_get_state(
u->manager->running_as == MANAGER_SYSTEM ? UNIT_FILE_SYSTEM : UNIT_FILE_USER,
NULL, basename(u->fragment_path));
NULL,
basename(u->fragment_path),
&u->unit_file_state);
if (r < 0)
u->unit_file_state = UNIT_FILE_BAD;
}
return u->unit_file_state;
}
@@ -3164,7 +3171,8 @@ int unit_get_unit_file_preset(Unit *u) {
if (u->unit_file_preset < 0 && u->fragment_path)
u->unit_file_preset = unit_file_query_preset(
u->manager->running_as == MANAGER_SYSTEM ? UNIT_FILE_SYSTEM : UNIT_FILE_USER,
NULL, basename(u->fragment_path));
NULL,
basename(u->fragment_path));
return u->unit_file_preset;
}

2322
src/shared/install.c
View File

File diff suppressed because it is too large Load Diff

40
src/shared/install.h
View File

@@ -25,13 +25,15 @@ typedef enum UnitFileScope UnitFileScope;
typedef enum UnitFileState UnitFileState;
typedef enum UnitFilePresetMode UnitFilePresetMode;
typedef enum UnitFileChangeType UnitFileChangeType;
typedef enum UnitFileType UnitFileType;
typedef struct UnitFileChange UnitFileChange;
typedef struct UnitFileList UnitFileList;
typedef struct UnitFileInstallInfo UnitFileInstallInfo;
#include "hashmap.h"
#include "unit-name.h"
#include "path-lookup.h"
#include "strv.h"
#include "unit-name.h"
enum UnitFileScope {
UNIT_FILE_SYSTEM,
@@ -51,7 +53,7 @@ enum UnitFileState {
UNIT_FILE_STATIC,
UNIT_FILE_DISABLED,
UNIT_FILE_INDIRECT,
UNIT_FILE_INVALID,
UNIT_FILE_BAD,
_UNIT_FILE_STATE_MAX,
_UNIT_FILE_STATE_INVALID = -1
};
@@ -82,6 +84,14 @@ struct UnitFileList {
UnitFileState state;
};
enum UnitFileType {
UNIT_FILE_TYPE_REGULAR,
UNIT_FILE_TYPE_SYMLINK,
UNIT_FILE_TYPE_MASKED,
_UNIT_FILE_TYPE_MAX,
_UNIT_FILE_TYPE_INVALID = -1,
};
struct UnitFileInstallInfo {
char *name;
char *path;
@@ -93,8 +103,26 @@ struct UnitFileInstallInfo {
char **also;
char *default_instance;
UnitFileType type;
char *symlink_target;
};
static inline bool UNIT_FILE_INSTALL_INFO_HAS_RULES(UnitFileInstallInfo *i) {
assert(i);
return !strv_isempty(i->aliases) ||
!strv_isempty(i->wanted_by) ||
!strv_isempty(i->required_by);
}
static inline bool UNIT_FILE_INSTALL_INFO_HAS_ALSO(UnitFileInstallInfo *i) {
assert(i);
return !strv_isempty(i->also);
}
int unit_file_enable(UnitFileScope scope, bool runtime, const char *root_dir, char **files, bool force, UnitFileChange **changes, unsigned *n_changes);
int unit_file_disable(UnitFileScope scope, bool runtime, const char *root_dir, char **files, UnitFileChange **changes, unsigned *n_changes);
int unit_file_reenable(UnitFileScope scope, bool runtime, const char *root_dir, char **files, bool force, UnitFileChange **changes, unsigned *n_changes);
@@ -105,14 +133,14 @@ int unit_file_mask(UnitFileScope scope, bool runtime, const char *root_dir, char
int unit_file_unmask(UnitFileScope scope, bool runtime, const char *root_dir, char **files, UnitFileChange **changes, unsigned *n_changes);
int unit_file_set_default(UnitFileScope scope, const char *root_dir, const char *file, bool force, UnitFileChange **changes, unsigned *n_changes);
int unit_file_get_default(UnitFileScope scope, const char *root_dir, char **name);
int unit_file_add_dependency(UnitFileScope scope, bool runtime, const char *root_dir, char **files, char *target, UnitDependency dep, bool force, UnitFileChange **changes, unsigned *n_changes);
int unit_file_add_dependency(UnitFileScope scope, bool runtime, const char *root_dir, char **files, const char *target, UnitDependency dep, bool force, UnitFileChange **changes, unsigned *n_changes);
UnitFileState unit_file_lookup_state(UnitFileScope scope, const char *root_dir,const LookupPaths *paths, const char *name);
UnitFileState unit_file_get_state(UnitFileScope scope, const char *root_dir, const char *filename);
int unit_file_lookup_state(UnitFileScope scope, const char *root_dir,const LookupPaths *paths, const char *name, UnitFileState *ret);
int unit_file_get_state(UnitFileScope scope, const char *root_dir, const char *filename, UnitFileState *ret);
int unit_file_get_list(UnitFileScope scope, const char *root_dir, Hashmap *h);
Hashmap* unit_file_list_free(Hashmap *h);
void unit_file_list_free(Hashmap *h);
int unit_file_changes_add(UnitFileChange **changes, unsigned *n_changes, UnitFileChangeType type, const char *path, const char *source);
void unit_file_changes_free(UnitFileChange *changes, unsigned n_changes);

6
src/systemctl/systemctl.c
View File

@@ -1335,7 +1335,7 @@ static void output_unit_file_list(const UnitFileList *units, unsigned c) {
UNIT_FILE_MASKED,
UNIT_FILE_MASKED_RUNTIME,
UNIT_FILE_DISABLED,
UNIT_FILE_INVALID)) {
UNIT_FILE_BAD)) {
on = ansi_highlight_red();
off = ansi_normal();
} else if (u->state == UNIT_FILE_ENABLED) {
@@ -5741,8 +5741,8 @@ static int unit_is_enabled(int argc, char *argv[], void *userdata) {
STRV_FOREACH(name, names) {
UnitFileState state;
state = unit_file_get_state(arg_scope, arg_root, *name);
if (state < 0)
r = unit_file_get_state(arg_scope, arg_root, *name, &state);
if (r < 0)
return log_error_errno(state, "Failed to get unit file state for %s: %m", *name);
if (IN_SET(state,

10
src/sysv-generator/sysv-generator.c
View File

@@ -742,6 +742,7 @@ static int fix_order(SysvStub *s, Hashmap *all_services) {
static int enumerate_sysv(const LookupPaths *lp, Hashmap *all_services) {
char **path;
int r;
assert(lp);
assert(all_services);
@@ -761,7 +762,6 @@ static int enumerate_sysv(const LookupPaths *lp, Hashmap *all_services) {
_cleanup_free_ char *fpath = NULL, *name = NULL;
_cleanup_(free_sysvstubp) SysvStub *service = NULL;
struct stat st;
int r;
if (fstatat(dirfd(d), de->d_name, &st, 0) < 0) {
log_warning_errno(errno, "stat() failed on %s/%s, ignoring: %m", *path, de->d_name);
@@ -781,8 +781,12 @@ static int enumerate_sysv(const LookupPaths *lp, Hashmap *all_services) {
if (hashmap_contains(all_services, name))
continue;
if (unit_file_lookup_state(UNIT_FILE_SYSTEM, NULL, lp, name) >= 0) {
log_debug("Native unit for %s already exists, skipping", name);
r = unit_file_lookup_state(UNIT_FILE_SYSTEM, NULL, lp, name, NULL);
if (r < 0 && r != -ENOENT) {
log_debug_errno(r, "Failed to detect whether %s exists, skipping: %m", name);
continue;
} else if (r >= 0) {
log_debug("Native unit for %s already exists, skipping.", name);
continue;
}

665
src/test/test-install-root.c Normal file
View File

File diff suppressed because it is too large Load Diff

71
src/test/test-install.c
View File

@@ -46,17 +46,19 @@ int main(int argc, char* argv[]) {
const char *const files2[] = { "/home/lennart/test.service", NULL };
UnitFileChange *changes = NULL;
unsigned n_changes = 0;
UnitFileState state = 0;
h = hashmap_new(&string_hash_ops);
r = unit_file_get_list(UNIT_FILE_SYSTEM, NULL, h);
assert_se(r == 0);
HASHMAP_FOREACH(p, h, i) {
UnitFileState s;
UnitFileState s = _UNIT_FILE_STATE_INVALID;
s = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(p->path));
r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(p->path), &s);
assert_se(p->state == s);
assert_se((r < 0 && p->state == UNIT_FILE_BAD) ||
(p->state == s));
fprintf(stderr, "%s (%s)\n",
p->path,
@@ -78,7 +80,9 @@ int main(int argc, char* argv[]) {
dump_changes(changes, n_changes);
unit_file_changes_free(changes, n_changes);
assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0]) == UNIT_FILE_ENABLED);
r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0], &state);
assert_se(r >= 0);
assert_se(state == UNIT_FILE_ENABLED);
log_error("disable");
@@ -91,7 +95,9 @@ int main(int argc, char* argv[]) {
dump_changes(changes, n_changes);
unit_file_changes_free(changes, n_changes);
assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0]) == UNIT_FILE_DISABLED);
r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0], &state);
assert_se(r >= 0);
assert_se(state == UNIT_FILE_DISABLED);
log_error("mask");
changes = NULL;
@@ -106,7 +112,9 @@ int main(int argc, char* argv[]) {
dump_changes(changes, n_changes);
unit_file_changes_free(changes, n_changes);
assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0]) == UNIT_FILE_MASKED);
r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0], &state);
assert_se(r >= 0);
assert_se(state == UNIT_FILE_MASKED);
log_error("unmask");
changes = NULL;
@@ -121,7 +129,9 @@ int main(int argc, char* argv[]) {
dump_changes(changes, n_changes);
unit_file_changes_free(changes, n_changes);
assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0]) == UNIT_FILE_DISABLED);
r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0], &state);
assert_se(r >= 0);
assert_se(state == UNIT_FILE_DISABLED);
log_error("mask");
changes = NULL;
@@ -133,7 +143,9 @@ int main(int argc, char* argv[]) {
dump_changes(changes, n_changes);
unit_file_changes_free(changes, n_changes);
assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0]) == UNIT_FILE_MASKED);
r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0], &state);
assert_se(r >= 0);
assert_se(state == UNIT_FILE_MASKED);
log_error("disable");
changes = NULL;
@@ -148,7 +160,9 @@ int main(int argc, char* argv[]) {
dump_changes(changes, n_changes);
unit_file_changes_free(changes, n_changes);
assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0]) == UNIT_FILE_MASKED);
r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0], &state);
assert_se(r >= 0);
assert_se(state == UNIT_FILE_MASKED);
log_error("umask");
changes = NULL;
@@ -160,7 +174,9 @@ int main(int argc, char* argv[]) {
dump_changes(changes, n_changes);
unit_file_changes_free(changes, n_changes);
assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0]) == UNIT_FILE_DISABLED);
r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0], &state);
assert_se(r >= 0);
assert_se(state == UNIT_FILE_DISABLED);
log_error("enable files2");
changes = NULL;
@@ -172,19 +188,22 @@ int main(int argc, char* argv[]) {
dump_changes(changes, n_changes);
unit_file_changes_free(changes, n_changes);
assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files2[0])) == UNIT_FILE_ENABLED);
r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files2[0]), &state);
assert_se(r >= 0);
assert_se(state == UNIT_FILE_ENABLED);
log_error("disable files2");
changes = NULL;
n_changes = 0;
r = unit_file_disable(UNIT_FILE_SYSTEM, false, NULL, (char**) files2, &changes, &n_changes);
r = unit_file_disable(UNIT_FILE_SYSTEM, false, NULL, STRV_MAKE(basename(files2[0])), &changes, &n_changes);
assert_se(r >= 0);
dump_changes(changes, n_changes);
unit_file_changes_free(changes, n_changes);
assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files2[0])) == _UNIT_FILE_STATE_INVALID);
r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files2[0]), &state);
assert_se(r < 0);
log_error("link files2");
changes = NULL;
@@ -196,19 +215,22 @@ int main(int argc, char* argv[]) {
dump_changes(changes, n_changes);
unit_file_changes_free(changes, n_changes);
assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files2[0])) == UNIT_FILE_LINKED);
r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files2[0]), &state);
assert_se(r >= 0);
assert_se(state == UNIT_FILE_LINKED);
log_error("disable files2");
changes = NULL;
n_changes = 0;
r = unit_file_disable(UNIT_FILE_SYSTEM, false, NULL, (char**) files2, &changes, &n_changes);
r = unit_file_disable(UNIT_FILE_SYSTEM, false, NULL, STRV_MAKE(basename(files2[0])), &changes, &n_changes);
assert_se(r >= 0);
dump_changes(changes, n_changes);
unit_file_changes_free(changes, n_changes);
assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files2[0])) == _UNIT_FILE_STATE_INVALID);
r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files2[0]), &state);
assert_se(r < 0);
log_error("link files2");
changes = NULL;
@@ -220,7 +242,9 @@ int main(int argc, char* argv[]) {
dump_changes(changes, n_changes);
unit_file_changes_free(changes, n_changes);
assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files2[0])) == UNIT_FILE_LINKED);
r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files2[0]), &state);
assert_se(r >= 0);
assert_se(state == UNIT_FILE_LINKED);
log_error("reenable files2");
changes = NULL;
@@ -232,19 +256,22 @@ int main(int argc, char* argv[]) {
dump_changes(changes, n_changes);
unit_file_changes_free(changes, n_changes);
assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files2[0])) == UNIT_FILE_ENABLED);
r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files2[0]), &state);
assert_se(r >= 0);
assert_se(state == UNIT_FILE_ENABLED);
log_error("disable files2");
changes = NULL;
n_changes = 0;
r = unit_file_disable(UNIT_FILE_SYSTEM, false, NULL, (char**) files2, &changes, &n_changes);
r = unit_file_disable(UNIT_FILE_SYSTEM, false, NULL, STRV_MAKE(basename(files2[0])), &changes, &n_changes);
assert_se(r >= 0);
dump_changes(changes, n_changes);
unit_file_changes_free(changes, n_changes);
assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files2[0])) == _UNIT_FILE_STATE_INVALID);
r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files2[0]), &state);
assert_se(r < 0);
log_error("preset files");
changes = NULL;
n_changes = 0;
@@ -255,7 +282,9 @@ int main(int argc, char* argv[]) {
dump_changes(changes, n_changes);
unit_file_changes_free(changes, n_changes);
assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files[0])) == UNIT_FILE_ENABLED);
r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files[0]), &state);
assert_se(r >= 0);
assert_se(state == UNIT_FILE_ENABLED);
return 0;
}