[systemd-devel] [PATCH] core/cryptsetup: Add WantsMountFor option to enable fallback to password request for crypt mounts. Signed-off-by: Przemek Rudy <prudy1 at o2.pl>
Przemyslaw Rudy
prudy1 at o2.pl
Mon Apr 28 05:26:18 PDT 2014
Hi Tom,
Sure, I'll get rid of this signed-off soon and re-send.
It has been tested with fedora 20 for all three options:
- with device inserted
- with device removed during startup but inserted before the mount timeout
- with device removed
Thanks
Przemek
On 04/28/2014 10:15 AM, Tom Gundersen wrote:
> On Mon, Apr 28, 2014 at 12:46 AM, Przemek Rudy <prudy1 at o2.pl> wrote:
>> This patch is a proposal for a problem with not falling back to password request
>> if the device with unlocking key for crypt volumes is not mounted for defined time.
>
> Looks good to me (but I didn't test it). Only one minor nit below.
>
> Also, you probably want to rewrite the subject of the patch (you can
> drop the signed-off-by as we don't use that here).
>
> Cheers,
>
> Tom
>
>> ---
>> src/core/dbus-unit.c | 1 +
>> src/core/load-fragment-gperf.gperf.m4 | 1 +
>> src/core/load-fragment.c | 47 +++++++++++
>> src/core/load-fragment.h | 1 +
>> src/core/manager.c | 15 ++++
>> src/core/manager.h | 6 ++
>> src/core/mount.c | 25 +++++-
>> src/core/unit.c | 144 ++++++++++++++++++++++++++++++++++
>> src/core/unit.h | 5 ++
>> src/cryptsetup/cryptsetup-generator.c | 3 +-
>> 10 files changed, 246 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c
>> index 07e7f20..31a35e9 100644
>> --- a/src/core/dbus-unit.c
>> +++ b/src/core/dbus-unit.c
>> @@ -516,6 +516,7 @@ const sd_bus_vtable bus_unit_vtable[] = {
>> SD_BUS_PROPERTY("ReloadPropagatedFrom", "as", property_get_dependencies, offsetof(Unit, dependencies[UNIT_RELOAD_PROPAGATED_FROM]), SD_BUS_VTABLE_PROPERTY_CONST),
>> SD_BUS_PROPERTY("JoinsNamespaceOf", "as", property_get_dependencies, offsetof(Unit, dependencies[UNIT_JOINS_NAMESPACE_OF]), SD_BUS_VTABLE_PROPERTY_CONST),
>> SD_BUS_PROPERTY("RequiresMountsFor", "as", NULL, offsetof(Unit, requires_mounts_for), SD_BUS_VTABLE_PROPERTY_CONST),
>> + SD_BUS_PROPERTY("WantsMountsFor", "as", NULL, offsetof(Unit, wants_mounts_for), SD_BUS_VTABLE_PROPERTY_CONST),
>> SD_BUS_PROPERTY("Documentation", "as", NULL, offsetof(Unit, documentation), SD_BUS_VTABLE_PROPERTY_CONST),
>> SD_BUS_PROPERTY("Description", "s", property_get_description, 0, SD_BUS_VTABLE_PROPERTY_CONST),
>> SD_BUS_PROPERTY("LoadState", "s", property_get_load_state, offsetof(Unit, load_state), SD_BUS_VTABLE_PROPERTY_CONST),
>> diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4
>> index 21bccbb..4e51866 100644
>> --- a/src/core/load-fragment-gperf.gperf.m4
>> +++ b/src/core/load-fragment-gperf.gperf.m4
>> @@ -139,6 +139,7 @@ Unit.PropagateReloadFrom, config_parse_unit_deps, UNIT_RELOAD
>> Unit.PartOf, config_parse_unit_deps, UNIT_PART_OF, 0
>> Unit.JoinsNamespaceOf, config_parse_unit_deps, UNIT_JOINS_NAMESPACE_OF, 0
>> Unit.RequiresMountsFor, config_parse_unit_requires_mounts_for, 0, 0
>> +Unit.WantsMountsFor, config_parse_unit_wants_mounts_for, 0, 0
>> Unit.StopWhenUnneeded, config_parse_bool, 0, offsetof(Unit, stop_when_unneeded)
>> Unit.RefuseManualStart, config_parse_bool, 0, offsetof(Unit, refuse_manual_start)
>> Unit.RefuseManualStop, config_parse_bool, 0, offsetof(Unit, refuse_manual_stop)
>> diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
>> index 3b36d15..24c1849 100644
>> --- a/src/core/load-fragment.c
>> +++ b/src/core/load-fragment.c
>> @@ -2048,6 +2048,52 @@ int config_parse_unit_requires_mounts_for(
>> return 0;
>> }
>>
>> +int config_parse_unit_wants_mounts_for(
>> + const char *unit,
>> + const char *filename,
>> + unsigned line,
>> + const char *section,
>> + unsigned section_line,
>> + const char *lvalue,
>> + int ltype,
>> + const char *rvalue,
>> + void *data,
>> + void *userdata) {
>> +
>> + Unit *u = userdata;
>> + char *state;
>> + size_t l;
>> + char *w;
>> +
>> + assert(filename);
>> + assert(lvalue);
>> + assert(rvalue);
>> + assert(data);
>> +
>> + FOREACH_WORD_QUOTED(w, l, rvalue, state) {
>> + int r;
>> + _cleanup_free_ char *n;
>> +
>> + n = strndup(w, l);
>> + if (!n)
>> + return log_oom();
>> +
>> + if (!utf8_is_valid(n)) {
>> + log_invalid_utf8(unit, LOG_ERR, filename, line, EINVAL, rvalue);
>> + continue;
>> + }
>> +
>> + r = unit_want_mounts_for(u, n);
>> + if (r < 0) {
>> + log_syntax(unit, LOG_ERR, filename, line, r,
>> + "Failed to add wanted mount for, ignoring: %s", rvalue);
>> + continue;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> int config_parse_documentation(const char *unit,
>> const char *filename,
>> unsigned line,
>> @@ -3422,6 +3468,7 @@ void unit_dump_config_items(FILE *f) {
>> { config_parse_nsec, "NANOSECONDS" },
>> { config_parse_namespace_path_strv, "PATH [...]" },
>> { config_parse_unit_requires_mounts_for, "PATH [...]" },
>> + { config_parse_unit_wants_mounts_for, "PATH [...]" },
>> { config_parse_exec_mount_flags, "MOUNTFLAG [...]" },
>> { config_parse_unit_string_printf, "STRING" },
>> { config_parse_trigger_unit, "UNIT" },
>> diff --git a/src/core/load-fragment.h b/src/core/load-fragment.h
>> index 242fd27..761d936 100644
>> --- a/src/core/load-fragment.h
>> +++ b/src/core/load-fragment.h
>> @@ -74,6 +74,7 @@ int config_parse_kill_mode(const char *unit, const char *filename, unsigned line
>> int config_parse_notify_access(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
>> int config_parse_failure_action(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
>> int config_parse_unit_requires_mounts_for(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
>> +int config_parse_unit_wants_mounts_for(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
>> int config_parse_syscall_filter(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
>> int config_parse_syscall_archs(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
>> int config_parse_syscall_errno(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
>> diff --git a/src/core/manager.c b/src/core/manager.c
>> index 5772f40..84ce11d 100644
>> --- a/src/core/manager.c
>> +++ b/src/core/manager.c
>> @@ -830,6 +830,9 @@ void manager_free(Manager *m) {
>> assert(hashmap_isempty(m->units_requiring_mounts_for));
>> hashmap_free(m->units_requiring_mounts_for);
>>
>> + assert(hashmap_isempty(m->units_want_mounts_for));
>> + hashmap_free(m->units_want_mounts_for);
>> +
>> free(m);
>> }
>>
>> @@ -2849,6 +2852,18 @@ Set *manager_get_units_requiring_mounts_for(Manager *m, const char *path) {
>> return hashmap_get(m->units_requiring_mounts_for, streq(p, "/") ? "" : p);
>> }
>>
>> +Set *manager_get_units_want_mounts_for(Manager *m, const char *path) {
>> + char p[strlen(path)+1];
>> +
>> + assert(m);
>> + assert(path);
>> +
>> + strcpy(p, path);
>> + path_kill_slashes(p);
>> +
>> + return hashmap_get(m->units_want_mounts_for, streq(p, "/") ? "" : p);
>> +}
>> +
>> const char *manager_get_runtime_prefix(Manager *m) {
>> assert(m);
>>
>> diff --git a/src/core/manager.h b/src/core/manager.h
>> index a3de351..ba5e688 100644
>> --- a/src/core/manager.h
>> +++ b/src/core/manager.h
>> @@ -269,6 +269,11 @@ struct Manager {
>> * value where Unit objects are contained. */
>> Hashmap *units_requiring_mounts_for;
>>
>> + /* This maps all possible path prefixes to the units want
>> + * them. It's a hashmap with a path string as key and a Set as
>> + * value where Unit objects are contained. */
>> + Hashmap *units_want_mounts_for;
>
> Shouldn't this be "units_wanting_mounts_for" to mimic
> "units_requiring_mounts_for"?
>
>> /* Reference to the kdbus bus control fd */
>> int kdbus_fd;
>> };
>> @@ -335,6 +340,7 @@ void manager_status_printf(Manager *m, bool ephemeral, const char *status, const
>> void manager_flip_auto_status(Manager *m, bool enable);
>>
>> Set *manager_get_units_requiring_mounts_for(Manager *m, const char *path);
>> +Set *manager_get_units_want_mounts_for(Manager *m, const char *path);
>>
>> const char *manager_get_runtime_prefix(Manager *m);
>>
>> diff --git a/src/core/mount.c b/src/core/mount.c
>> index a979837..64d6914 100644
>> --- a/src/core/mount.c
>> +++ b/src/core/mount.c
>> @@ -264,7 +264,7 @@ static int mount_add_mount_links(Mount *m) {
>> return r;
>> }
>>
>> - /* Adds in links to other units that use this path or paths
>> + /* Adds in links to other units that use (require) this path or paths
>> * further down in the hierarchy */
>> s = manager_get_units_requiring_mounts_for(UNIT(m)->manager, m->where);
>> SET_FOREACH(other, s, i) {
>> @@ -287,6 +287,29 @@ static int mount_add_mount_links(Mount *m) {
>> }
>> }
>>
>> + /* Adds in links to other units that use (want) this path or paths
>> + * further down in the hierarchy */
>> + s = manager_get_units_want_mounts_for(UNIT(m)->manager, m->where);
>> + SET_FOREACH(other, s, i) {
>> +
>> + if (other->load_state != UNIT_LOADED)
>> + continue;
>> +
>> + if (other == UNIT(m))
>> + continue;
>> +
>> + r = unit_add_dependency(other, UNIT_AFTER, UNIT(m), true);
>> + if (r < 0)
>> + return r;
>> +
>> + if (UNIT(m)->fragment_path) {
>> + /* If we have fragment configuration, then make this dependency required */
>> + r = unit_add_dependency(other, UNIT_WANTS, UNIT(m), true);
>> + if (r < 0)
>> + return r;
>> + }
>> + }
>> +
>> return 0;
>> }
>>
>> diff --git a/src/core/unit.c b/src/core/unit.c
>> index 6ac359e..f7c03f7 100644
>> --- a/src/core/unit.c
>> +++ b/src/core/unit.c
>> @@ -428,6 +428,34 @@ static void unit_free_requires_mounts_for(Unit *u) {
>> u->requires_mounts_for = NULL;
>> }
>>
>> +static void unit_free_wants_mounts_for(Unit *u) {
>> + char **j;
>> +
>> + STRV_FOREACH(j, u->wants_mounts_for) {
>> + char s[strlen(*j) + 1];
>> +
>> + PATH_FOREACH_PREFIX_MORE(s, *j) {
>> + char *y;
>> + Set *x;
>> +
>> + x = hashmap_get2(u->manager->units_want_mounts_for, s, (void**) &y);
>> + if (!x)
>> + continue;
>> +
>> + set_remove(x, u);
>> +
>> + if (set_isempty(x)) {
>> + hashmap_remove(u->manager->units_want_mounts_for, y);
>> + free(y);
>> + set_free(x);
>> + }
>> + }
>> + }
>> +
>> + strv_free(u->wants_mounts_for);
>> + u->wants_mounts_for = NULL;
>> +}
>> +
>> static void unit_done(Unit *u) {
>> ExecContext *ec;
>> CGroupContext *cc;
>> @@ -464,6 +492,7 @@ void unit_free(Unit *u) {
>> unit_done(u);
>>
>> unit_free_requires_mounts_for(u);
>> + unit_free_wants_mounts_for(u);
>>
>> SET_FOREACH(t, u->names, i)
>> hashmap_remove_value(u->manager->units, t, u);
>> @@ -885,6 +914,16 @@ void unit_dump(Unit *u, FILE *f, const char *prefix) {
>> fputs("\n", f);
>> }
>>
>> + if (!strv_isempty(u->wants_mounts_for)) {
>> + fprintf(f,
>> + "%s\tWantsMountsFor:", prefix);
>> +
>> + STRV_FOREACH(j, u->wants_mounts_for)
>> + fprintf(f, " %s", *j);
>> +
>> + fputs("\n", f);
>> + }
>> +
>> if (u->load_state == UNIT_LOADED) {
>>
>> fprintf(f,
>> @@ -1068,6 +1107,35 @@ static int unit_add_mount_dependencies(Unit *u) {
>> }
>> }
>>
>> + STRV_FOREACH(i, u->wants_mounts_for) {
>> + char prefix[strlen(*i) + 1];
>> +
>> + PATH_FOREACH_PREFIX_MORE(prefix, *i) {
>> + Unit *m;
>> +
>> + r = manager_get_unit_by_path(u->manager, prefix, ".mount", &m);
>> + if (r < 0)
>> + return r;
>> + if (r == 0)
>> + continue;
>> + if (m == u)
>> + continue;
>> +
>> + if (m->load_state != UNIT_LOADED)
>> + continue;
>> +
>> + r = unit_add_dependency(u, UNIT_AFTER, m, true);
>> + if (r < 0)
>> + return r;
>> +
>> + if (m->fragment_path) {
>> + r = unit_add_dependency(u, UNIT_WANTS, m, true);
>> + if (r < 0)
>> + return r;
>> + }
>> + }
>> + }
>> +
>> return 0;
>> }
>>
>> @@ -3289,6 +3357,82 @@ int unit_require_mounts_for(Unit *u, const char *path) {
>> return 0;
>> }
>>
>> +int unit_want_mounts_for(Unit *u, const char *path) {
>> + char prefix[strlen(path) + 1], *p;
>> + int r;
>> +
>> + assert(u);
>> + assert(path);
>> +
>> + /* Registers a unit for want a certain path and all its
>> + * prefixes. We keep a simple array of these paths in the
>> + * unit, since its usually short. However, we build a prefix
>> + * table for all possible prefixes so that new appearing mount
>> + * units can easily determine which units to make themselves a
>> + * dependency of. */
>> +
>> + if (!path_is_absolute(path))
>> + return -EINVAL;
>> +
>> + p = strdup(path);
>> + if (!p)
>> + return -ENOMEM;
>> +
>> + path_kill_slashes(p);
>> +
>> + if (!path_is_safe(p)) {
>> + free(p);
>> + return -EPERM;
>> + }
>> +
>> + if (strv_contains(u->wants_mounts_for, p)) {
>> + free(p);
>> + return 0;
>> + }
>> +
>> + r = strv_consume(&u->wants_mounts_for, p);
>> + if (r < 0)
>> + return r;
>> +
>> + PATH_FOREACH_PREFIX_MORE(prefix, p) {
>> + Set *x;
>> +
>> + x = hashmap_get(u->manager->units_want_mounts_for, prefix);
>> + if (!x) {
>> + char *q;
>> +
>> + if (!u->manager->units_want_mounts_for) {
>> + u->manager->units_want_mounts_for = hashmap_new(string_hash_func, string_compare_func);
>> + if (!u->manager->units_want_mounts_for)
>> + return -ENOMEM;
>> + }
>> +
>> + q = strdup(prefix);
>> + if (!q)
>> + return -ENOMEM;
>> +
>> + x = set_new(NULL, NULL);
>> + if (!x) {
>> + free(q);
>> + return -ENOMEM;
>> + }
>> +
>> + r = hashmap_put(u->manager->units_want_mounts_for, q, x);
>> + if (r < 0) {
>> + free(q);
>> + set_free(x);
>> + return r;
>> + }
>> + }
>> +
>> + r = set_put(x, u);
>> + if (r < 0)
>> + return r;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> int unit_setup_exec_runtime(Unit *u) {
>> ExecRuntime **rt;
>> size_t offset;
>> diff --git a/src/core/unit.h b/src/core/unit.h
>> index 3e61067..0db0b91 100644
>> --- a/src/core/unit.h
>> +++ b/src/core/unit.h
>> @@ -143,6 +143,7 @@ struct Unit {
>> Set *dependencies[_UNIT_DEPENDENCY_MAX];
>>
>> char **requires_mounts_for;
>> + char **wants_mounts_for;
>>
>> char *description;
>> char **documentation;
>> @@ -189,6 +190,9 @@ struct Unit {
>> /* All units which have requires_mounts_for set */
>> LIST_FIELDS(Unit, has_requires_mounts_for);
>>
>> + /* All units which have wantss_mounts_for set */
>> + LIST_FIELDS(Unit, has_wants_mounts_for);
>> +
>> /* Load queue */
>> LIST_FIELDS(Unit, load_queue);
>>
>> @@ -625,6 +629,7 @@ int unit_kill_context(Unit *u, KillContext *c, bool sigkill, pid_t main_pid, pid
>> int unit_make_transient(Unit *u);
>>
>> int unit_require_mounts_for(Unit *u, const char *path);
>> +int unit_want_mounts_for(Unit *u, const char *path);
>>
>> const char *unit_active_state_to_string(UnitActiveState i) _const_;
>> UnitActiveState unit_active_state_from_string(const char *s) _pure_;
>> diff --git a/src/cryptsetup/cryptsetup-generator.c b/src/cryptsetup/cryptsetup-generator.c
>> index f4eeb2a..ce7ae92 100644
>> --- a/src/cryptsetup/cryptsetup-generator.c
>> +++ b/src/cryptsetup/cryptsetup-generator.c
>> @@ -153,7 +153,8 @@ static int create_disk(
>>
>> fprintf(f, "After=%1$s\nRequires=%1$s\n", dd);
>> } else
>> - fprintf(f, "RequiresMountsFor=%s\n", password);
>> + /* Do not use 'RequiresMountsFor=' here to allow fallback to password in case the key device is not available */
>> + fprintf(f, "WantsMountsFor=%s\n", password);
>> }
>> }
>>
>> --
>> 1.9.0
>>
>> _______________________________________________
>> systemd-devel mailing list
>> systemd-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>
More information about the systemd-devel
mailing list