Skip to content

Commit 56a13a4

Browse files
keszybzkeszybz
committed
pid1: create ro private tmp dirs when /tmp or /var/tmp is read-only
Read-only /var/tmp is more likely, because it's backed by a real device. /tmp is (by default) backed by tmpfs, but it doesn't have to be. In both cases the same consideration applies. If we boot with read-only /var/tmp, any unit with PrivateTmp=yes would fail because we cannot create the subdir under /var/tmp to mount the private directory. But many services actually don't require /var/tmp (either because they only use it occasionally, or because they only use /tmp, or even because they don't use the temporary directories at all, and PrivateTmp=yes is used to isolate them from the rest of the system). To handle both cases let's create a read-only directory under /run/systemd and mount it as the private /tmp or /var/tmp. (Read-only to not fool the service into dumping too much data in /run.) $ sudo systemd-run -t -p PrivateTmp=yes bash Running as unit: run-u14.service Press ^] three times within 1s to disconnect TTY. [root@workstation /]# ls -l /tmp/ total 0 [root@workstation /]# ls -l /var/tmp/ total 0 [root@workstation /]# touch /tmp/f [root@workstation /]# touch /var/tmp/f touch: cannot touch '/var/tmp/f': Read-only file system This commit has more changes than I like to put in one commit, but it's touching all the same paths so it's hard to split. exec_runtime_make() was using the wrong cleanup function, so the directory would be left behind on error.
1 parent cbc056c commit 56a13a4

File tree

6 files changed

+174
-125
lines changed

6 files changed

+174
-125
lines changed

‎src/core/execute.c‎

Lines changed: 68 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -2592,7 +2592,7 @@ static int apply_mount_namespace(
25922592
char **error_path) {
25932593

25942594
_cleanup_strv_free_ char **empty_directories = NULL;
2595-
char *tmp = NULL, *var = NULL;
2595+
const char *tmp_dir = NULL, *var_tmp_dir = NULL;
25962596
const char *root_dir = NULL, *root_image = NULL;
25972597
NamespaceInfo ns_info;
25982598
bool needs_sandboxing;
@@ -2617,13 +2617,19 @@ static int apply_mount_namespace(
26172617
if (needs_sandboxing) {
26182618
/* The runtime struct only contains the parent of the private /tmp,
26192619
* which is non-accessible to world users. Inside of it there's a /tmp
2620-
* that is sticky, and that's the one we want to use here. */
2620+
* that is sticky, and that's the one we want to use here.
2621+
* This does not apply when we are using /run/systemd/empty as fallback. */
26212622

26222623
if (context->private_tmp && runtime) {
2623-
if (runtime->tmp_dir)
2624-
tmp = strjoina(runtime->tmp_dir, "/tmp");
2625-
if (runtime->var_tmp_dir)
2626-
var = strjoina(runtime->var_tmp_dir, "/tmp");
2624+
if (streq_ptr(runtime->tmp_dir, RUN_SYSTEMD_EMPTY))
2625+
tmp_dir = runtime->tmp_dir;
2626+
else if (runtime->tmp_dir)
2627+
tmp_dir = strjoina(runtime->tmp_dir, "/tmp");
2628+
2629+
if (streq_ptr(runtime->var_tmp_dir, RUN_SYSTEMD_EMPTY))
2630+
var_tmp_dir = runtime->var_tmp_dir;
2631+
else if (runtime->tmp_dir)
2632+
var_tmp_dir = strjoina(runtime->var_tmp_dir, "/tmp");
26272633
}
26282634

26292635
ns_info = (NamespaceInfo) {
@@ -2661,8 +2667,8 @@ static int apply_mount_namespace(
26612667
n_bind_mounts,
26622668
context->temporary_filesystems,
26632669
context->n_temporary_filesystems,
2664-
tmp,
2665-
var,
2670+
tmp_dir,
2671+
var_tmp_dir,
26662672
context->log_namespace,
26672673
needs_sandboxing ? context->protect_home : PROTECT_HOME_NO,
26682674
needs_sandboxing ? context->protect_system : PROTECT_SYSTEM_NO,
@@ -5347,28 +5353,25 @@ static ExecRuntime* exec_runtime_free(ExecRuntime *rt, bool destroy) {
53475353
(void) hashmap_remove(rt->manager->exec_runtime_by_id, rt->id);
53485354

53495355
/* When destroy is true, then rm_rf tmp_dir and var_tmp_dir. */
5350-
if (destroy && rt->tmp_dir) {
5356+
5357+
if (destroy && rt->tmp_dir && !streq(rt->tmp_dir, RUN_SYSTEMD_EMPTY)) {
53515358
log_debug("Spawning thread to nuke %s", rt->tmp_dir);
53525359

53535360
r = asynchronous_job(remove_tmpdir_thread, rt->tmp_dir);
5354-
if (r < 0) {
5361+
if (r < 0)
53555362
log_warning_errno(r, "Failed to nuke %s: %m", rt->tmp_dir);
5356-
free(rt->tmp_dir);
5357-
}
5358-
5359-
rt->tmp_dir = NULL;
5363+
else
5364+
rt->tmp_dir = NULL;
53605365
}
53615366

5362-
if (destroy && rt->var_tmp_dir) {
5367+
if (destroy && rt->var_tmp_dir && !streq(rt->var_tmp_dir, RUN_SYSTEMD_EMPTY)) {
53635368
log_debug("Spawning thread to nuke %s", rt->var_tmp_dir);
53645369

53655370
r = asynchronous_job(remove_tmpdir_thread, rt->var_tmp_dir);
5366-
if (r < 0) {
5371+
if (r < 0)
53675372
log_warning_errno(r, "Failed to nuke %s: %m", rt->var_tmp_dir);
5368-
free(rt->var_tmp_dir);
5369-
}
5370-
5371-
rt->var_tmp_dir = NULL;
5373+
else
5374+
rt->var_tmp_dir = NULL;
53725375
}
53735376

53745377
rt->id = mfree(rt->id);
@@ -5382,16 +5385,22 @@ static void exec_runtime_freep(ExecRuntime **rt) {
53825385
(void) exec_runtime_free(*rt, false);
53835386
}
53845387

5385-
static int exec_runtime_allocate(ExecRuntime **ret) {
5388+
static int exec_runtime_allocate(ExecRuntime **ret, const char *id) {
5389+
_cleanup_free_ char *id_copy = NULL;
53865390
ExecRuntime *n;
53875391

53885392
assert(ret);
53895393

5394+
id_copy = strdup(id);
5395+
if (!id_copy)
5396+
return -ENOMEM;
5397+
53905398
n = new(ExecRuntime, 1);
53915399
if (!n)
53925400
return -ENOMEM;
53935401

53945402
*n = (ExecRuntime) {
5403+
.id = TAKE_PTR(id_copy),
53955404
.netns_storage_socket = { -1, -1 },
53965405
};
53975406

@@ -5402,9 +5411,9 @@ static int exec_runtime_allocate(ExecRuntime **ret) {
54025411
static int exec_runtime_add(
54035412
Manager *m,
54045413
const char *id,
5405-
const char *tmp_dir,
5406-
const char *var_tmp_dir,
5407-
const int netns_storage_socket[2],
5414+
char **tmp_dir,
5415+
char **var_tmp_dir,
5416+
int netns_storage_socket[2],
54085417
ExecRuntime **ret) {
54095418

54105419
_cleanup_(exec_runtime_freep) ExecRuntime *rt = NULL;
@@ -5413,51 +5422,40 @@ static int exec_runtime_add(
54135422
assert(m);
54145423
assert(id);
54155424

5425+
/* tmp_dir, var_tmp_dir, netns_storage_socket fds are donated on success */
5426+
54165427
r = hashmap_ensure_allocated(&m->exec_runtime_by_id, &string_hash_ops);
54175428
if (r < 0)
54185429
return r;
54195430

5420-
r = exec_runtime_allocate(&rt);
5431+
r = exec_runtime_allocate(&rt, id);
54215432
if (r < 0)
54225433
return r;
54235434

5424-
rt->id = strdup(id);
5425-
if (!rt->id)
5426-
return -ENOMEM;
5427-
5428-
if (tmp_dir) {
5429-
rt->tmp_dir = strdup(tmp_dir);
5430-
if (!rt->tmp_dir)
5431-
return -ENOMEM;
5435+
r = hashmap_put(m->exec_runtime_by_id, rt->id, rt);
5436+
if (r < 0)
5437+
return r;
54325438

5433-
/* When tmp_dir is set, then we require var_tmp_dir is also set. */
5434-
assert(var_tmp_dir);
5435-
rt->var_tmp_dir = strdup(var_tmp_dir);
5436-
if (!rt->var_tmp_dir)
5437-
return -ENOMEM;
5438-
}
5439+
assert(!!rt->tmp_dir == !!rt->var_tmp_dir); /* We require both to be set together */
5440+
rt->tmp_dir = TAKE_PTR(*tmp_dir);
5441+
rt->var_tmp_dir = TAKE_PTR(*var_tmp_dir);
54395442

54405443
if (netns_storage_socket) {
5441-
rt->netns_storage_socket[0] = netns_storage_socket[0];
5442-
rt->netns_storage_socket[1] = netns_storage_socket[1];
5444+
rt->netns_storage_socket[0] = TAKE_FD(netns_storage_socket[0]);
5445+
rt->netns_storage_socket[1] = TAKE_FD(netns_storage_socket[1]);
54435446
}
54445447

5445-
r = hashmap_put(m->exec_runtime_by_id, rt->id, rt);
5446-
if (r < 0)
5447-
return r;
5448-
54495448
rt->manager = m;
54505449

54515450
if (ret)
54525451
*ret = rt;
5453-
54545452
/* do not remove created ExecRuntime object when the operation succeeds. */
5455-
rt = NULL;
5453+
TAKE_PTR(rt);
54565454
return 0;
54575455
}
54585456

54595457
static int exec_runtime_make(Manager *m, const ExecContext *c, const char *id, ExecRuntime **ret) {
5460-
_cleanup_free_ char *tmp_dir = NULL, *var_tmp_dir = NULL;
5458+
_cleanup_(namespace_cleanup_tmpdirp) char *tmp_dir = NULL, *var_tmp_dir = NULL;
54615459
_cleanup_close_pair_ int netns_storage_socket[2] = { -1, -1 };
54625460
int r;
54635461

@@ -5483,12 +5481,10 @@ static int exec_runtime_make(Manager *m, const ExecContext *c, const char *id, E
54835481
return -errno;
54845482
}
54855483

5486-
r = exec_runtime_add(m, id, tmp_dir, var_tmp_dir, netns_storage_socket, ret);
5484+
r = exec_runtime_add(m, id, &tmp_dir, &var_tmp_dir, netns_storage_socket, ret);
54875485
if (r < 0)
54885486
return r;
54895487

5490-
/* Avoid cleanup */
5491-
netns_storage_socket[0] = netns_storage_socket[1] = -1;
54925488
return 1;
54935489
}
54945490

@@ -5606,14 +5602,10 @@ int exec_runtime_deserialize_compat(Unit *u, const char *key, const char *value,
56065602

56075603
rt = hashmap_get(u->manager->exec_runtime_by_id, u->id);
56085604
if (!rt) {
5609-
r = exec_runtime_allocate(&rt_create);
5605+
r = exec_runtime_allocate(&rt_create, u->id);
56105606
if (r < 0)
56115607
return log_oom();
56125608

5613-
rt_create->id = strdup(u->id);
5614-
if (!rt_create->id)
5615-
return log_oom();
5616-
56175609
rt = rt_create;
56185610
}
56195611

@@ -5670,15 +5662,16 @@ int exec_runtime_deserialize_compat(Unit *u, const char *key, const char *value,
56705662
rt_create->manager = u->manager;
56715663

56725664
/* Avoid cleanup */
5673-
rt_create = NULL;
5665+
TAKE_PTR(rt_create);
56745666
}
56755667

56765668
return 1;
56775669
}
56785670

5679-
void exec_runtime_deserialize_one(Manager *m, const char *value, FDSet *fds) {
5680-
char *id = NULL, *tmp_dir = NULL, *var_tmp_dir = NULL;
5681-
int r, fd0 = -1, fd1 = -1;
5671+
int exec_runtime_deserialize_one(Manager *m, const char *value, FDSet *fds) {
5672+
_cleanup_free_ char *tmp_dir = NULL, *var_tmp_dir = NULL;
5673+
char *id = NULL;
5674+
int r, fdpair[] = {-1, -1};
56825675
const char *p, *v = value;
56835676
size_t n;
56845677

@@ -5695,7 +5688,9 @@ void exec_runtime_deserialize_one(Manager *m, const char *value, FDSet *fds) {
56955688
v = startswith(p, "tmp-dir=");
56965689
if (v) {
56975690
n = strcspn(v, " ");
5698-
tmp_dir = strndupa(v, n);
5691+
tmp_dir = strndup(v, n);
5692+
if (!tmp_dir)
5693+
return log_oom();
56995694
if (v[n] != ' ')
57005695
goto finalize;
57015696
p = v + n + 1;
@@ -5704,7 +5699,9 @@ void exec_runtime_deserialize_one(Manager *m, const char *value, FDSet *fds) {
57045699
v = startswith(p, "var-tmp-dir=");
57055700
if (v) {
57065701
n = strcspn(v, " ");
5707-
var_tmp_dir = strndupa(v, n);
5702+
var_tmp_dir = strndup(v, n);
5703+
if (!var_tmp_dir)
5704+
return log_oom();
57085705
if (v[n] != ' ')
57095706
goto finalize;
57105707
p = v + n + 1;
@@ -5716,11 +5713,9 @@ void exec_runtime_deserialize_one(Manager *m, const char *value, FDSet *fds) {
57165713

57175714
n = strcspn(v, " ");
57185715
buf = strndupa(v, n);
5719-
if (safe_atoi(buf, &fd0) < 0 || !fdset_contains(fds, fd0)) {
5720-
log_debug("Unable to process exec-runtime netns fd specification.");
5721-
return;
5722-
}
5723-
fd0 = fdset_remove(fds, fd0);
5716+
if (safe_atoi(buf, &fdpair[0]) < 0 || !fdset_contains(fds, fdpair[0]))
5717+
return log_debug("Unable to process exec-runtime netns fd specification.");
5718+
fdpair[0] = fdset_remove(fds, fdpair[0]);
57245719
if (v[n] != ' ')
57255720
goto finalize;
57265721
p = v + n + 1;
@@ -5732,18 +5727,16 @@ void exec_runtime_deserialize_one(Manager *m, const char *value, FDSet *fds) {
57325727

57335728
n = strcspn(v, " ");
57345729
buf = strndupa(v, n);
5735-
if (safe_atoi(buf, &fd1) < 0 || !fdset_contains(fds, fd1)) {
5736-
log_debug("Unable to process exec-runtime netns fd specification.");
5737-
return;
5738-
}
5739-
fd1 = fdset_remove(fds, fd1);
5730+
if (safe_atoi(buf, &fdpair[1]) < 0 || !fdset_contains(fds, fdpair[1]))
5731+
return log_debug("Unable to process exec-runtime netns fd specification.");
5732+
fdpair[1] = fdset_remove(fds, fdpair[1]);
57405733
}
57415734

57425735
finalize:
5743-
5744-
r = exec_runtime_add(m, id, tmp_dir, var_tmp_dir, (int[]) { fd0, fd1 }, NULL);
5736+
r = exec_runtime_add(m, id, &tmp_dir, &var_tmp_dir, fdpair, NULL);
57455737
if (r < 0)
5746-
log_debug_errno(r, "Failed to add exec-runtime: %m");
5738+
return log_debug_errno(r, "Failed to add exec-runtime: %m");
5739+
return 0;
57475740
}
57485741

57495742
void exec_runtime_vacuum(Manager *m) {

‎src/core/execute.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,7 @@ ExecRuntime *exec_runtime_unref(ExecRuntime *r, bool destroy);
405405

406406
int exec_runtime_serialize(const Manager *m, FILE *f, FDSet *fds);
407407
int exec_runtime_deserialize_compat(Unit *u, const char *key, const char *value, FDSet *fds);
408-
void exec_runtime_deserialize_one(Manager *m, const char *value, FDSet *fds);
408+
int exec_runtime_deserialize_one(Manager *m, const char *value, FDSet *fds);
409409
void exec_runtime_vacuum(Manager *m);
410410

411411
void exec_params_clear(ExecParameters *p);

‎src/core/manager.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3685,7 +3685,7 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
36853685
else if ((val = startswith(l, "destroy-ipc-gid=")))
36863686
manager_deserialize_gid_refs_one(m, val);
36873687
else if ((val = startswith(l, "exec-runtime=")))
3688-
exec_runtime_deserialize_one(m, val, fds);
3688+
(void) exec_runtime_deserialize_one(m, val, fds);
36893689
else if ((val = startswith(l, "subscribed="))) {
36903690

36913691
if (strv_extend(&m->deserialized_subscribed, val) < 0)

0 commit comments

Comments
 (0)