Skip to content

Commit

Permalink
core: rework unit name validation and manipulation logic
Browse files Browse the repository at this point in the history
A variety of changes:

- Make sure all our calls distuingish OOM from other errors if OOM is
  not the only error possible.

- Be much stricter when parsing escaped paths, do not accept trailing or
  leading escaped slashes.

- Change unit validation to take a bit mask for allowing plain names,
  instance names or template names or an combination thereof.

- Refuse manipulating invalid unit name
  • Loading branch information
poettering committed May 5, 2015
1 parent 6442185 commit 7410616
Show file tree
Hide file tree
Showing 34 changed files with 1,033 additions and 834 deletions.
19 changes: 9 additions & 10 deletions src/core/automount.c
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,9 @@ static int automount_add_default_dependencies(Automount *a) {
}

static int automount_verify(Automount *a) {
bool b;
_cleanup_free_ char *e = NULL;
int r;

assert(a);

if (UNIT(a)->load_state != UNIT_LOADED)
Expand All @@ -179,13 +180,11 @@ static int automount_verify(Automount *a) {
return -EINVAL;
}

e = unit_name_from_path(a->where, ".automount");
if (!e)
return -ENOMEM;

b = unit_has_name(UNIT(a), e);
r = unit_name_from_path(a->where, ".automount", &e);
if (r < 0)
return log_unit_error(UNIT(a)->id, "Failed to generate unit name from path: %m");

if (!b) {
if (!unit_has_name(UNIT(a), e)) {
log_unit_error(UNIT(a)->id, "%s's Where setting doesn't match unit name. Refusing.", UNIT(a)->id);
return -EINVAL;
}
Expand All @@ -209,9 +208,9 @@ static int automount_load(Unit *u) {
Unit *x;

if (!a->where) {
a->where = unit_name_to_path(u->id);
if (!a->where)
return -ENOMEM;
r = unit_name_to_path(u->id, &a->where);
if (r < 0)
return r;
}

path_kill_slashes(a->where);
Expand Down
6 changes: 3 additions & 3 deletions src/core/busname.c
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,9 @@ static int busname_add_extras(BusName *n) {
assert(n);

if (!n->name) {
n->name = unit_name_to_prefix(u->id);
if (!n->name)
return -ENOMEM;
r = unit_name_to_prefix(u->id, &n->name);
if (r < 0)
return r;
}

if (!u->description) {
Expand Down
4 changes: 2 additions & 2 deletions src/core/dbus-unit.c
Original file line number Diff line number Diff line change
Expand Up @@ -940,7 +940,7 @@ static int bus_unit_set_transient_property(
if (r < 0)
return r;

if (!unit_name_is_valid(s, TEMPLATE_INVALID) || !endswith(s, ".slice"))
if (!unit_name_is_valid(s, UNIT_NAME_PLAIN) || !endswith(s, ".slice"))
return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid slice name %s", s);

if (isempty(s)) {
Expand Down Expand Up @@ -988,7 +988,7 @@ static int bus_unit_set_transient_property(
return r;

while ((r = sd_bus_message_read(message, "s", &other)) > 0) {
if (!unit_name_is_valid(other, TEMPLATE_INVALID))
if (!unit_name_is_valid(other, UNIT_NAME_PLAIN|UNIT_NAME_INSTANCE))
return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid unit name %s", other);

if (mode != UNIT_CHECK) {
Expand Down
19 changes: 10 additions & 9 deletions src/core/device.c
Original file line number Diff line number Diff line change
Expand Up @@ -279,9 +279,9 @@ static int device_add_udev_wants(Unit *u, struct udev_device *dev) {
memcpy(e, word, l);
e[l] = 0;

n = unit_name_mangle(e, MANGLE_NOGLOB);
if (!n)
return log_oom();
r = unit_name_mangle(e, UNIT_NAME_NOGLOB, &n);
if (r < 0)
return log_unit_error_errno(u->id, r, "Failed to mangle unit name: %m");

r = unit_add_dependency_by_name(u, UNIT_WANTS, n, NULL, true);
if (r < 0)
Expand All @@ -308,9 +308,9 @@ static int device_setup_unit(Manager *m, struct udev_device *dev, const char *pa
if (!sysfs)
return 0;

e = unit_name_from_path(path, ".device");
if (!e)
return log_oom();
r = unit_name_from_path(path, ".device", &e);
if (r < 0)
return log_unit_error_errno(u->id, r, "Failed to generate device name: %m");

u = manager_get_unit(m, e);

Expand Down Expand Up @@ -491,16 +491,17 @@ static int device_update_found_by_sysfs(Manager *m, const char *sysfs, bool add,
static int device_update_found_by_name(Manager *m, const char *path, bool add, DeviceFound found, bool now) {
_cleanup_free_ char *e = NULL;
Unit *u;
int r;

assert(m);
assert(path);

if (found == DEVICE_NOT_FOUND)
return 0;

e = unit_name_from_path(path, ".device");
if (!e)
return log_oom();
r = unit_name_from_path(path, ".device", &e);
if (r < 0)
return log_error_errno(r, "Failed to generate unit name from device path: %m");

u = manager_get_unit(m, e);
if (!u)
Expand Down
16 changes: 8 additions & 8 deletions src/core/load-fragment.c
Original file line number Diff line number Diff line change
Expand Up @@ -3394,7 +3394,7 @@ static int open_follow(char **filename, FILE **_f, Set *names, char **_final) {
* unit name. */
name = basename(*filename);

if (unit_name_is_valid(name, TEMPLATE_VALID)) {
if (unit_name_is_valid(name, UNIT_NAME_ANY)) {

id = set_get(names, name);
if (!id) {
Expand Down Expand Up @@ -3642,11 +3642,11 @@ int unit_load_fragment(Unit *u) {

/* Look for a template */
if (u->load_state == UNIT_STUB && u->instance) {
_cleanup_free_ char *k;
_cleanup_free_ char *k = NULL;

k = unit_name_template(u->id);
if (!k)
return -ENOMEM;
r = unit_name_template(u->id, &k);
if (r < 0)
return r;

r = load_from_path(u, k);
if (r < 0)
Expand All @@ -3659,9 +3659,9 @@ int unit_load_fragment(Unit *u) {
if (t == u->id)
continue;

z = unit_name_template(t);
if (!z)
return -ENOMEM;
r = unit_name_template(t, &z);
if (r < 0)
return r;

r = load_from_path(u, z);
if (r < 0)
Expand Down
17 changes: 9 additions & 8 deletions src/core/manager.c
Original file line number Diff line number Diff line change
Expand Up @@ -1306,7 +1306,7 @@ int manager_load_unit_prepare(

t = unit_name_to_type(name);

if (t == _UNIT_TYPE_INVALID || !unit_name_is_valid(name, TEMPLATE_INVALID))
if (t == _UNIT_TYPE_INVALID || !unit_name_is_valid(name, UNIT_NAME_PLAIN|UNIT_NAME_INSTANCE))
return sd_bus_error_setf(e, SD_BUS_ERROR_INVALID_ARGS, "Unit name %s is not valid.", name);

ret = manager_get_unit(m, name);
Expand Down Expand Up @@ -2093,7 +2093,7 @@ void manager_send_unit_audit(Manager *m, Unit *u, int type, bool success) {
#ifdef HAVE_AUDIT
_cleanup_free_ char *p = NULL;
const char *msg;
int audit_fd;
int audit_fd, r;

audit_fd = get_audit_fd();
if (audit_fd < 0)
Expand All @@ -2110,9 +2110,9 @@ void manager_send_unit_audit(Manager *m, Unit *u, int type, bool success) {
if (u->type != UNIT_SERVICE)
return;

p = unit_name_to_prefix_and_instance(u->id);
if (!p) {
log_oom();
r = unit_name_to_prefix_and_instance(u->id, &p);
if (r < 0) {
log_error_errno(r, "Failed to extract prefix and instance of unit name: %m");
return;
}

Expand Down Expand Up @@ -3027,15 +3027,16 @@ void manager_status_printf(Manager *m, StatusType type, const char *status, cons
int manager_get_unit_by_path(Manager *m, const char *path, const char *suffix, Unit **_found) {
_cleanup_free_ char *p = NULL;
Unit *found;
int r;

assert(m);
assert(path);
assert(suffix);
assert(_found);

p = unit_name_from_path(path, suffix);
if (!p)
return -ENOMEM;
r = unit_name_from_path(path, suffix, &p);
if (r < 0)
return r;

found = manager_get_unit(m, p);
if (!found) {
Expand Down
23 changes: 11 additions & 12 deletions src/core/mount.c
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,7 @@ static int mount_add_default_dependencies(Mount *m) {

static int mount_verify(Mount *m) {
_cleanup_free_ char *e = NULL;
bool b;
int r;

assert(m);

Expand All @@ -445,12 +445,11 @@ static int mount_verify(Mount *m) {
if (!m->from_fragment && !m->from_proc_self_mountinfo)
return -ENOENT;

e = unit_name_from_path(m->where, ".mount");
if (!e)
return -ENOMEM;
r = unit_name_from_path(m->where, ".mount", &e);
if (r < 0)
return log_unit_error_errno(UNIT(m)->id, r, "Failed to generate unit name from mount path: %m");

b = unit_has_name(UNIT(m), e);
if (!b) {
if (!unit_has_name(UNIT(m), e)) {
log_unit_error(UNIT(m)->id, "%s's Where= setting doesn't match unit name. Refusing.", UNIT(m)->id);
return -EINVAL;
}
Expand Down Expand Up @@ -483,9 +482,9 @@ static int mount_add_extras(Mount *m) {
m->from_fragment = true;

if (!m->where) {
m->where = unit_name_to_path(u->id);
if (!m->where)
return -ENOMEM;
r = unit_name_to_path(u->id, &m->where);
if (r < 0)
return r;
}

path_kill_slashes(m->where);
Expand Down Expand Up @@ -1419,9 +1418,9 @@ static int mount_setup_unit(
if (!is_path(where))
return 0;

e = unit_name_from_path(where, ".mount");
if (!e)
return -ENOMEM;
r = unit_name_from_path(where, ".mount", &e);
if (r < 0)
return r;

u = manager_get_unit(m, e);
if (!u) {
Expand Down
4 changes: 2 additions & 2 deletions src/core/snapshot.c
Original file line number Diff line number Diff line change
Expand Up @@ -201,10 +201,10 @@ int snapshot_create(Manager *m, const char *name, bool cleanup, sd_bus_error *e,
assert(_s);

if (name) {
if (!unit_name_is_valid(name, TEMPLATE_INVALID))
if (!unit_name_is_valid(name, UNIT_NAME_PLAIN))
return sd_bus_error_setf(e, SD_BUS_ERROR_INVALID_ARGS, "Unit name %s is not valid.", name);

if (unit_name_to_type(name) != UNIT_SNAPSHOT)
if (!endswith(name, ".snapshot"))
return sd_bus_error_setf(e, SD_BUS_ERROR_INVALID_ARGS, "Unit name %s lacks snapshot suffix.", name);

if (manager_get_unit(m, name))
Expand Down
23 changes: 11 additions & 12 deletions src/core/socket.c
Original file line number Diff line number Diff line change
Expand Up @@ -196,12 +196,15 @@ int socket_instantiate_service(Socket *s) {
* here. For Accept=no this is mostly a NOP since the service
* is figured out at load time anyway. */

if (UNIT_DEREF(s->service) || !s->accept)
if (UNIT_DEREF(s->service))
return 0;

prefix = unit_name_to_prefix(UNIT(s)->id);
if (!prefix)
return -ENOMEM;
if (!s->accept)
return 0;

r = unit_name_to_prefix(UNIT(s)->id, &prefix);
if (r < 0)
return r;

if (asprintf(&name, "%s@%u.service", prefix, s->n_accepted) < 0)
return -ENOMEM;
Expand Down Expand Up @@ -1836,17 +1839,13 @@ static void socket_enter_running(Socket *s, int cfd) {
return;
}

prefix = unit_name_to_prefix(UNIT(s)->id);
if (!prefix) {
r = -ENOMEM;
r = unit_name_to_prefix(UNIT(s)->id, &prefix);
if (r < 0)
goto fail;
}

name = unit_name_build(prefix, instance, ".service");
if (!name) {
r = -ENOMEM;
r = unit_name_build(prefix, instance, ".service", &name);
if (r < 0)
goto fail;
}

r = unit_add_name(UNIT_DEREF(s->service), name);
if (r < 0)
Expand Down
39 changes: 20 additions & 19 deletions src/core/swap.c
Original file line number Diff line number Diff line change
Expand Up @@ -222,18 +222,17 @@ static int swap_add_default_dependencies(Swap *s) {
}

static int swap_verify(Swap *s) {
bool b;
_cleanup_free_ char *e = NULL;
int r;

if (UNIT(s)->load_state != UNIT_LOADED)
return 0;

e = unit_name_from_path(s->what, ".swap");
if (!e)
return log_oom();
r = unit_name_from_path(s->what, ".swap", &e);
if (r < 0)
return log_unit_error_errno(UNIT(s)->id, r, "%s: failed to generate unit name from path: %m", UNIT(s)->id);

b = unit_has_name(UNIT(s), e);
if (!b) {
if (!unit_has_name(UNIT(s), e)) {
log_unit_error(UNIT(s)->id, "%s: Value of \"What\" and unit name do not match, not loading.", UNIT(s)->id);
return -EINVAL;
}
Expand Down Expand Up @@ -289,8 +288,11 @@ static int swap_load(Unit *u) {
s->what = strdup(s->parameters_fragment.what);
else if (s->parameters_proc_swaps.what)
s->what = strdup(s->parameters_proc_swaps.what);
else
s->what = unit_name_to_path(u->id);
else {
r = unit_name_to_path(u->id, &s->what);
if (r < 0)
return r;
}

if (!s->what)
return -ENOMEM;
Expand Down Expand Up @@ -355,9 +357,9 @@ static int swap_setup_unit(
assert(what);
assert(what_proc_swaps);

e = unit_name_from_path(what, ".swap");
if (!e)
return log_oom();
r = unit_name_from_path(what, ".swap", &e);
if (r < 0)
return log_unit_error_errno(u->id, r, "Failed to generate unit name from path: %m");

u = manager_get_unit(m, e);

Expand Down Expand Up @@ -1329,9 +1331,9 @@ int swap_process_device_new(Manager *m, struct udev_device *dev) {
if (!dn)
return 0;

e = unit_name_from_path(dn, ".swap");
if (!e)
return -ENOMEM;
r = unit_name_from_path(dn, ".swap", &e);
if (r < 0)
return r;

s = hashmap_get(m->units, e);
if (s)
Expand All @@ -1340,15 +1342,14 @@ int swap_process_device_new(Manager *m, struct udev_device *dev) {
first = udev_device_get_devlinks_list_entry(dev);
udev_list_entry_foreach(item, first) {
_cleanup_free_ char *n = NULL;
int q;

n = unit_name_from_path(udev_list_entry_get_name(item), ".swap");
if (!n)
return -ENOMEM;
q = unit_name_from_path(udev_list_entry_get_name(item), ".swap", &n);
if (q < 0)
return q;

s = hashmap_get(m->units, n);
if (s) {
int q;

q = swap_set_devnode(s, dn);
if (q < 0)
r = q;
Expand Down
Loading

0 comments on commit 7410616

Please sign in to comment.