picnoir
/
Systemd
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

pid1: when creating service directories, don't chown existing files (#8181) 

This partially reverts 3536f49e8f and
3536f49e8f.

When the user is dynamic, and we are setting up state, cache, or logs dirs,
behaviour is unchanged, we always do a recursive chown. This is necessary
because the user number might change between invocations.

But when setting up a directory for non-dynamic user, or a runtime directory
for a dynamic user, do any ownership or mode changes only when the directory
is initially created. Nothing says that the files under those directories have
to be all recursively owned by our user. This restores behaviour before
3536f49e8f, so modifications to the state of
the runtime directory persist between ExecStartPre's and ExecStart's, and even
longer in case the directory is persistent.

I think it _would_ be a nice property if setting a user would automatically
propagate to ownership of any Runtime/Logs/Cache directories. But this is
incompatible with another nice property, namely preserving changes to those
directories made by an admin, and with allowing change of ownership of files
in those directories by the service (e.g. to allow other users to access them).
Of the two, I think the second property is more important. Also, it's backwards
compatible.

https://bugzilla.redhat.com/show_bug.cgi?id=1508495

There is no need to chmod a directory we just created, so move that step
up into a branch. After that, 'effective' is only used once, so get rid of
it too.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2018-02-22 11:30:59 +01:00 committed by Lennart Poettering
parent a4896a1f14
commit 30c81ce2ce
1 changed files with 9 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
src/core/execute.c
View File

@ -2012,7 +2012,6 @@ static int setup_exec_directory(
STRV_FOREACH(rt, context->directories[type].paths) {
_cleanup_free_ char *p = NULL, *pp = NULL;
const char *effective;
p = strjoin(params->prefix[type], "/", *rt);
if (!p) {
@ -2106,20 +2105,17 @@ static int setup_exec_directory(
if (r < 0)
goto fail;
effective = pp;
/* Lock down the access mode */
if (chmod(pp, context->directories[type].mode) < 0) {
r = -errno;
goto fail;
}
} else {
r = mkdir_label(p, context->directories[type].mode);
if (r < 0 && r != -EEXIST)
if (r == -EEXIST)
continue;
if (r < 0)
goto fail;
effective = p;
}
/* First lock down the access mode */
if (chmod(effective, context->directories[type].mode) < 0) {
r = -errno;
goto fail;
}
/* Don't change the owner of the configuration directory, as in the common case it is not written to by
@ -2128,7 +2124,7 @@ static int setup_exec_directory(
continue;
/* Then, change the ownership of the whole tree, if necessary */
r = path_chown_recursive(effective, uid, gid);
r = path_chown_recursive(pp ?: p, uid, gid);
if (r < 0)
goto fail;
}