Skip to content

Commit 7410616

Browse files
poetteringpoettering
committed
core: rework unit name validation and manipulation logic
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
1 parent 6442185 commit 7410616

File tree

34 files changed

+1033
-834
lines changed

34 files changed

+1033
-834
lines changed

‎src/core/automount.c‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -167,8 +167,9 @@ static int automount_add_default_dependencies(Automount *a) {
167167
}
168168

169169
static int automount_verify(Automount *a) {
170-
bool b;
171170
_cleanup_free_ char *e = NULL;
171+
int r;
172+
172173
assert(a);
173174

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

182-
e = unit_name_from_path(a->where, ".automount");
183-
if (!e)
184-
return -ENOMEM;
185-
186-
b = unit_has_name(UNIT(a), e);
183+
r = unit_name_from_path(a->where, ".automount", &e);
184+
if (r < 0)
185+
return log_unit_error(UNIT(a)->id, "Failed to generate unit name from path: %m");
187186

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

211210
if (!a->where) {
212-
a->where = unit_name_to_path(u->id);
213-
if (!a->where)
214-
return -ENOMEM;
211+
r = unit_name_to_path(u->id, &a->where);
212+
if (r < 0)
213+
return r;
215214
}
216215

217216
path_kill_slashes(a->where);

‎src/core/busname.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -163,9 +163,9 @@ static int busname_add_extras(BusName *n) {
163163
assert(n);
164164

165165
if (!n->name) {
166-
n->name = unit_name_to_prefix(u->id);
167-
if (!n->name)
168-
return -ENOMEM;
166+
r = unit_name_to_prefix(u->id, &n->name);
167+
if (r < 0)
168+
return r;
169169
}
170170

171171
if (!u->description) {

‎src/core/dbus-unit.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -940,7 +940,7 @@ static int bus_unit_set_transient_property(
940940
if (r < 0)
941941
return r;
942942

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

946946
if (isempty(s)) {
@@ -988,7 +988,7 @@ static int bus_unit_set_transient_property(
988988
return r;
989989

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

994994
if (mode != UNIT_CHECK) {

‎src/core/device.c‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -279,9 +279,9 @@ static int device_add_udev_wants(Unit *u, struct udev_device *dev) {
279279
memcpy(e, word, l);
280280
e[l] = 0;
281281

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

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

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

315315
u = manager_get_unit(m, e);
316316

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

495496
assert(m);
496497
assert(path);
497498

498499
if (found == DEVICE_NOT_FOUND)
499500
return 0;
500501

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

505506
u = manager_get_unit(m, e);
506507
if (!u)

‎src/core/load-fragment.c‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3394,7 +3394,7 @@ static int open_follow(char **filename, FILE **_f, Set *names, char **_final) {
33943394
* unit name. */
33953395
name = basename(*filename);
33963396

3397-
if (unit_name_is_valid(name, TEMPLATE_VALID)) {
3397+
if (unit_name_is_valid(name, UNIT_NAME_ANY)) {
33983398

33993399
id = set_get(names, name);
34003400
if (!id) {
@@ -3642,11 +3642,11 @@ int unit_load_fragment(Unit *u) {
36423642

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

3647-
k = unit_name_template(u->id);
3648-
if (!k)
3649-
return -ENOMEM;
3647+
r = unit_name_template(u->id, &k);
3648+
if (r < 0)
3649+
return r;
36503650

36513651
r = load_from_path(u, k);
36523652
if (r < 0)
@@ -3659,9 +3659,9 @@ int unit_load_fragment(Unit *u) {
36593659
if (t == u->id)
36603660
continue;
36613661

3662-
z = unit_name_template(t);
3663-
if (!z)
3664-
return -ENOMEM;
3662+
r = unit_name_template(t, &z);
3663+
if (r < 0)
3664+
return r;
36653665

36663666
r = load_from_path(u, z);
36673667
if (r < 0)

‎src/core/manager.c‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1306,7 +1306,7 @@ int manager_load_unit_prepare(
13061306

13071307
t = unit_name_to_type(name);
13081308

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

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

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

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

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

30313032
assert(m);
30323033
assert(path);
30333034
assert(suffix);
30343035
assert(_found);
30353036

3036-
p = unit_name_from_path(path, suffix);
3037-
if (!p)
3038-
return -ENOMEM;
3037+
r = unit_name_from_path(path, suffix, &p);
3038+
if (r < 0)
3039+
return r;
30393040

30403041
found = manager_get_unit(m, p);
30413042
if (!found) {

‎src/core/mount.c‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,7 @@ static int mount_add_default_dependencies(Mount *m) {
435435

436436
static int mount_verify(Mount *m) {
437437
_cleanup_free_ char *e = NULL;
438-
bool b;
438+
int r;
439439

440440
assert(m);
441441

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

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

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

485484
if (!m->where) {
486-
m->where = unit_name_to_path(u->id);
487-
if (!m->where)
488-
return -ENOMEM;
485+
r = unit_name_to_path(u->id, &m->where);
486+
if (r < 0)
487+
return r;
489488
}
490489

491490
path_kill_slashes(m->where);
@@ -1419,9 +1418,9 @@ static int mount_setup_unit(
14191418
if (!is_path(where))
14201419
return 0;
14211420

1422-
e = unit_name_from_path(where, ".mount");
1423-
if (!e)
1424-
return -ENOMEM;
1421+
r = unit_name_from_path(where, ".mount", &e);
1422+
if (r < 0)
1423+
return r;
14251424

14261425
u = manager_get_unit(m, e);
14271426
if (!u) {

‎src/core/snapshot.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,10 +201,10 @@ int snapshot_create(Manager *m, const char *name, bool cleanup, sd_bus_error *e,
201201
assert(_s);
202202

203203
if (name) {
204-
if (!unit_name_is_valid(name, TEMPLATE_INVALID))
204+
if (!unit_name_is_valid(name, UNIT_NAME_PLAIN))
205205
return sd_bus_error_setf(e, SD_BUS_ERROR_INVALID_ARGS, "Unit name %s is not valid.", name);
206206

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

210210
if (manager_get_unit(m, name))

‎src/core/socket.c‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -196,12 +196,15 @@ int socket_instantiate_service(Socket *s) {
196196
* here. For Accept=no this is mostly a NOP since the service
197197
* is figured out at load time anyway. */
198198

199-
if (UNIT_DEREF(s->service) || !s->accept)
199+
if (UNIT_DEREF(s->service))
200200
return 0;
201201

202-
prefix = unit_name_to_prefix(UNIT(s)->id);
203-
if (!prefix)
204-
return -ENOMEM;
202+
if (!s->accept)
203+
return 0;
204+
205+
r = unit_name_to_prefix(UNIT(s)->id, &prefix);
206+
if (r < 0)
207+
return r;
205208

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

1839-
prefix = unit_name_to_prefix(UNIT(s)->id);
1840-
if (!prefix) {
1841-
r = -ENOMEM;
1842+
r = unit_name_to_prefix(UNIT(s)->id, &prefix);
1843+
if (r < 0)
18421844
goto fail;
1843-
}
18441845

1845-
name = unit_name_build(prefix, instance, ".service");
1846-
if (!name) {
1847-
r = -ENOMEM;
1846+
r = unit_name_build(prefix, instance, ".service", &name);
1847+
if (r < 0)
18481848
goto fail;
1849-
}
18501849

18511850
r = unit_add_name(UNIT_DEREF(s->service), name);
18521851
if (r < 0)

‎src/core/swap.c‎

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -222,18 +222,17 @@ static int swap_add_default_dependencies(Swap *s) {
222222
}
223223

224224
static int swap_verify(Swap *s) {
225-
bool b;
226225
_cleanup_free_ char *e = NULL;
226+
int r;
227227

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

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

235-
b = unit_has_name(UNIT(s), e);
236-
if (!b) {
235+
if (!unit_has_name(UNIT(s), e)) {
237236
log_unit_error(UNIT(s)->id, "%s: Value of \"What\" and unit name do not match, not loading.", UNIT(s)->id);
238237
return -EINVAL;
239238
}
@@ -289,8 +288,11 @@ static int swap_load(Unit *u) {
289288
s->what = strdup(s->parameters_fragment.what);
290289
else if (s->parameters_proc_swaps.what)
291290
s->what = strdup(s->parameters_proc_swaps.what);
292-
else
293-
s->what = unit_name_to_path(u->id);
291+
else {
292+
r = unit_name_to_path(u->id, &s->what);
293+
if (r < 0)
294+
return r;
295+
}
294296

295297
if (!s->what)
296298
return -ENOMEM;
@@ -355,9 +357,9 @@ static int swap_setup_unit(
355357
assert(what);
356358
assert(what_proc_swaps);
357359

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

362364
u = manager_get_unit(m, e);
363365

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

1332-
e = unit_name_from_path(dn, ".swap");
1333-
if (!e)
1334-
return -ENOMEM;
1334+
r = unit_name_from_path(dn, ".swap", &e);
1335+
if (r < 0)
1336+
return r;
13351337

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

1344-
n = unit_name_from_path(udev_list_entry_get_name(item), ".swap");
1345-
if (!n)
1346-
return -ENOMEM;
1347+
q = unit_name_from_path(udev_list_entry_get_name(item), ".swap", &n);
1348+
if (q < 0)
1349+
return q;
13471350

13481351
s = hashmap_get(m->units, n);
13491352
if (s) {
1350-
int q;
1351-
13521353
q = swap_set_devnode(s, dn);
13531354
if (q < 0)
13541355
r = q;

0 commit comments

Comments
 (0)