machine-credential: introduce MachineCredentialContext

This allows more straightforward memory management and
the use of static destructor.

Requested (by me) in https://github.com/systemd/systemd/pull/30143#discussion_r1401980763

diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 1f79d75eef24233a038b351155af08af62430d31..1902ee8aed5dad658dcb89c900682c3e4c6ea146 100644 (file)
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -229,8 +229,7 @@ static DeviceNode* arg_extra_nodes = NULL;
 static size_t arg_n_extra_nodes = 0;
 static char **arg_sysctl = NULL;
 static ConsoleMode arg_console_mode = _CONSOLE_MODE_INVALID;
-static MachineCredential *arg_credentials = NULL;
-static size_t arg_n_credentials = 0;
+static MachineCredentialContext arg_credentials = {};
 static char **arg_bind_user = NULL;
 static bool arg_suppress_sync = false;
 static char *arg_settings_filename = NULL;
@@ -266,6 +265,7 @@ STATIC_DESTRUCTOR_REGISTER(arg_syscall_deny_list, strv_freep);
 #if HAVE_SECCOMP
 STATIC_DESTRUCTOR_REGISTER(arg_seccomp, seccomp_releasep);
 #endif
+STATIC_DESTRUCTOR_REGISTER(arg_credentials, machine_credential_context_done);
 STATIC_DESTRUCTOR_REGISTER(arg_cpu_set, cpu_set_reset);
 STATIC_DESTRUCTOR_REGISTER(arg_sysctl, strv_freep);
 STATIC_DESTRUCTOR_REGISTER(arg_bind_user, strv_freep);
@@ -1568,7 +1568,7 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
 
                 case ARG_SET_CREDENTIAL:
-                        r = machine_credential_set(&arg_credentials, &arg_n_credentials, optarg);
+                        r = machine_credential_set(&arg_credentials, optarg);
                         if (r < 0)
                                 return r;
 
@@ -1576,7 +1576,7 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
 
                 case ARG_LOAD_CREDENTIAL:
-                        r = machine_credential_load(&arg_credentials, &arg_n_credentials, optarg);
+                        r = machine_credential_load(&arg_credentials, optarg);
                         if (r < 0)
                                 return r;
 
@@ -2367,7 +2367,7 @@ static int setup_credentials(const char *root) {
         const char *q;
         int r;
 
-        if (arg_n_credentials <= 0)
+        if (arg_credentials.n_credentials == 0)
                 return 0;
 
         r = userns_mkdir(root, "/run/host", 0755, 0, 0);
@@ -2383,11 +2383,11 @@ static int setup_credentials(const char *root) {
         if (r < 0)
                 return r;
 
-        for (size_t i = 0; i < arg_n_credentials; i++) {
+        FOREACH_ARRAY(cred, arg_credentials.credentials, arg_credentials.n_credentials) {
                 _cleanup_free_ char *j = NULL;
                 _cleanup_close_ int fd = -EBADF;
 
-                j = path_join(q, arg_credentials[i].id);
+                j = path_join(q, cred->id);
                 if (!j)
                         return log_oom();
 
@@ -2395,7 +2395,7 @@ static int setup_credentials(const char *root) {
                 if (fd < 0)
                         return log_error_errno(errno, "Failed to create credential file %s: %m", j);
 
-                r = loop_write(fd, arg_credentials[i].data, arg_credentials[i].size);
+                r = loop_write(fd, cred->data, cred->size);
                 if (r < 0)
                         return log_error_errno(r, "Failed to write credential to file %s: %m", j);
 
@@ -3406,7 +3406,7 @@ static int inner_child(
         if (asprintf(envp + n_env++, "NOTIFY_SOCKET=%s", NSPAWN_NOTIFY_SOCKET_PATH) < 0)
                 return log_oom();
 
-        if (arg_n_credentials > 0) {
+        if (arg_credentials.n_credentials > 0) {
                 envp[n_env] = strdup("CREDENTIALS_DIRECTORY=/run/host/credentials");
                 if (!envp[n_env])
                         return log_oom();
@@ -5858,7 +5858,6 @@ finish:
         expose_port_free_all(arg_expose_ports);
         rlimit_free_all(arg_rlimit);
         device_node_array_free(arg_extra_nodes, arg_n_extra_nodes);
-        machine_credential_free_all(arg_credentials, arg_n_credentials);
 
         if (r < 0)
                 return r;

diff --git a/src/shared/machine-credential.c b/src/shared/machine-credential.c
index 17f7afc4a0fdf8cf0a8186b4f6e9a87a776fe087..c1e76e468e5285f1669ee2b88789578297310da2 100644 (file)
--- a/src/shared/machine-credential.c
+++ b/src/shared/machine-credential.c
@@ -19,76 +19,82 @@ static void machine_credential_done(MachineCredential *cred) {
         cred->size = 0;
 }
 
-void machine_credential_free_all(MachineCredential *creds, size_t n) {
-        assert(creds || n == 0);
+void machine_credential_context_done(MachineCredentialContext *ctx) {
+        assert(ctx);
 
-        FOREACH_ARRAY(cred, creds, n)
+        FOREACH_ARRAY(cred, ctx->credentials, ctx->n_credentials)
                 machine_credential_done(cred);
 
-        free(creds);
+        free(ctx->credentials);
 }
 
-int machine_credential_set(MachineCredential **credentials, size_t *n_credentials, const char *cred_string) {
-        _cleanup_free_ char *word = NULL, *data = NULL;
+bool machine_credentials_contains(const MachineCredentialContext *ctx, const char *id) {
+        assert(ctx);
+        assert(id);
+
+        FOREACH_ARRAY(cred, ctx->credentials, ctx->n_credentials)
+                if (streq(cred->id, id))
+                        return true;
+
+        return false;
+}
+
+int machine_credential_set(MachineCredentialContext *ctx, const char *cred_str) {
+        _cleanup_(machine_credential_done) MachineCredential cred = {};
         ssize_t l;
         int r;
-        const char *p = ASSERT_PTR(cred_string);
 
-        assert(credentials && n_credentials);
-        assert(*credentials || *n_credentials == 0);
+        assert(ctx);
 
-        r = extract_first_word(&p, &word, ":", EXTRACT_DONT_COALESCE_SEPARATORS);
+        const char *p = ASSERT_PTR(cred_str);
+
+        r = extract_first_word(&p, &cred.id, ":", EXTRACT_DONT_COALESCE_SEPARATORS);
         if (r < 0)
                 return log_error_errno(r, "Failed to parse --set-credential= parameter: %m");
         if (r == 0 || !p)
-                return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Missing value for --set-credential=: %s", cred_string);
+                return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
+                                       "Missing value for --set-credential=: %s", cred_str);
 
-        if (!credential_name_valid(word))
-                return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Credential name is not valid: %s", word);
+        if (!credential_name_valid(cred.id))
+                return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Credential name is not valid: %s", cred.id);
 
-        FOREACH_ARRAY(cred, *credentials, *n_credentials)
-                if (streq(cred->id, word))
-                        return log_error_errno(SYNTHETIC_ERRNO(EEXIST), "Duplicate credential '%s', refusing.", word);
+        if (machine_credentials_contains(ctx, cred.id))
+                return log_error_errno(SYNTHETIC_ERRNO(EEXIST), "Duplicate credential '%s', refusing.", cred.id);
 
-        l = cunescape(p, UNESCAPE_ACCEPT_NUL, &data);
+        l = cunescape(p, UNESCAPE_ACCEPT_NUL, &cred.data);
         if (l < 0)
                 return log_error_errno(l, "Failed to unescape credential data: %s", p);
+        cred.size = l;
 
-        if (!GREEDY_REALLOC(*credentials, *n_credentials + 1))
+        if (!GREEDY_REALLOC(ctx->credentials, ctx->n_credentials + 1))
                 return log_oom();
 
-        (*credentials)[(*n_credentials)++] = (MachineCredential) {
-                .id = TAKE_PTR(word),
-                .data = TAKE_PTR(data),
-                .size = l,
-        };
+        ctx->credentials[ctx->n_credentials++] = TAKE_STRUCT(cred);
 
         return 0;
 }
 
-int machine_credential_load(MachineCredential **credentials, size_t *n_credentials, const char *cred_path) {
+int machine_credential_load(MachineCredentialContext *ctx, const char *cred_path) {
+        _cleanup_(machine_credential_done) MachineCredential cred = {};
+        _cleanup_free_ char *path_alloc = NULL;
         ReadFullFileFlags flags = READ_FULL_FILE_SECURE;
-        _cleanup_(erase_and_freep) char *data = NULL;
-        _cleanup_free_ char *word = NULL, *j = NULL;
-        const char *p = ASSERT_PTR(cred_path);
-        size_t size;
         int r;
 
-        assert(credentials && n_credentials);
-        assert(*credentials || *n_credentials == 0);
+        assert(ctx);
+
+        const char *p = ASSERT_PTR(cred_path);
 
-        r = extract_first_word(&p, &word, ":", EXTRACT_DONT_COALESCE_SEPARATORS);
+        r = extract_first_word(&p, &cred.id, ":", EXTRACT_DONT_COALESCE_SEPARATORS);
         if (r < 0)
                 return log_error_errno(r, "Failed to parse --load-credential= parameter: %m");
         if (r == 0 || !p)
                 return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Missing value for --load-credential=: %s", cred_path);
 
-        if (!credential_name_valid(word))
-                return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Credential name is not valid: %s", word);
+        if (!credential_name_valid(cred.id))
+                return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Credential name is not valid: %s", cred.id);
 
-        FOREACH_ARRAY(cred, *credentials, *n_credentials)
-                if (streq(cred->id, word))
-                        return log_error_errno(SYNTHETIC_ERRNO(EEXIST), "Duplicate credential '%s', refusing.", word);
+        if (machine_credentials_contains(ctx, cred.id))
+                return log_error_errno(SYNTHETIC_ERRNO(EEXIST), "Duplicate credential '%s', refusing.", cred.id);
 
         if (is_path(p) && path_is_valid(p))
                 flags |= READ_FULL_FILE_CONNECT_SOCKET;
@@ -97,31 +103,29 @@ int machine_credential_load(MachineCredential **credentials, size_t *n_credentia
 
                 r = get_credentials_dir(&e);
                 if (r < 0)
-                        return log_error_errno(r, "Credential not available (no credentials passed at all): %s", word);
+                        return log_error_errno(r,
+                                               "Credential not available (no credentials passed at all): %s", cred.id);
 
-                j = path_join(e, p);
-                if (!j)
+                path_alloc = path_join(e, p);
+                if (!path_alloc)
                         return log_oom();
 
-                p = j;
+                p = path_alloc;
         } else
-                return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Credential source appears to be neither a valid path nor a credential name: %s", p);
+                return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
+                                       "Credential source appears to be neither a valid path nor a credential name: %s", p);
 
         r = read_full_file_full(AT_FDCWD, p, UINT64_MAX, SIZE_MAX,
                                 flags,
                                 NULL,
-                                &data, &size);
+                                &cred.data, &cred.size);
         if (r < 0)
                 return log_error_errno(r, "Failed to read credential '%s': %m", p);
 
-        if (!GREEDY_REALLOC(*credentials, *n_credentials + 1))
+        if (!GREEDY_REALLOC(ctx->credentials, ctx->n_credentials + 1))
                 return log_oom();
 
-        (*credentials)[(*n_credentials)++] = (MachineCredential) {
-                .id = TAKE_PTR(word),
-                .data = TAKE_PTR(data),
-                .size = size,
-        };
+        ctx->credentials[ctx->n_credentials++] = TAKE_STRUCT(cred);
 
         return 0;
 }

diff --git a/src/shared/machine-credential.h b/src/shared/machine-credential.h
index c9044a2edcc7fa903fa44e25746f70ffcd8a6d66..182f077fe7e4fbc01af5a5485dd4f7ed4d54e52e 100644 (file)
--- a/src/shared/machine-credential.h
+++ b/src/shared/machine-credential.h
@@ -5,10 +5,18 @@
 
 typedef struct MachineCredential {
         char *id;
-        void *data;
+        char *data;
         size_t size;
 } MachineCredential;
 
-void machine_credential_free_all(MachineCredential *creds, size_t n);
-int machine_credential_set(MachineCredential **credentials, size_t *n_credentials, const char *cred_string);
-int machine_credential_load(MachineCredential **credentials, size_t *n_credentials, const char *cred_path);
+typedef struct MachineCredentialContext {
+        MachineCredential *credentials;
+        size_t n_credentials;
+} MachineCredentialContext;
+
+void machine_credential_context_done(MachineCredentialContext *ctx);
+
+bool machine_credentials_contains(const MachineCredentialContext *ctx, const char *id);
+
+int machine_credential_set(MachineCredentialContext *ctx, const char *cred_str);
+int machine_credential_load(MachineCredentialContext *ctx, const char *cred_path);

diff --git a/src/vmspawn/vmspawn.c b/src/vmspawn/vmspawn.c
index ebae681893ed4a4123d2cfefbab49fd10ab90c6f..4ccda8bae8b8d8d1b5526c30ae8e961bdadd7d9c 100644 (file)
--- a/src/vmspawn/vmspawn.c
+++ b/src/vmspawn/vmspawn.c
@@ -45,8 +45,7 @@ static int arg_qemu_vsock = -1;
 static uint64_t arg_vsock_cid = UINT64_MAX;
 static bool arg_qemu_gui = false;
 static int arg_secure_boot = -1;
-static MachineCredential *arg_credentials = NULL;
-static size_t arg_n_credentials = 0;
+static MachineCredentialContext arg_credentials = {};
 static SettingsMask arg_settings_mask = 0;
 static char **arg_parameters = NULL;
 
@@ -54,6 +53,7 @@ STATIC_DESTRUCTOR_REGISTER(arg_image, freep);
 STATIC_DESTRUCTOR_REGISTER(arg_machine, freep);
 STATIC_DESTRUCTOR_REGISTER(arg_qemu_smp, freep);
 STATIC_DESTRUCTOR_REGISTER(arg_parameters, strv_freep);
+STATIC_DESTRUCTOR_REGISTER(arg_credentials, machine_credential_context_done);
 
 static int help(void) {
         _cleanup_free_ char *link = NULL;
@@ -224,7 +224,7 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
 
                 case ARG_SET_CREDENTIAL: {
-                        r = machine_credential_set(&arg_credentials, &arg_n_credentials, optarg);
+                        r = machine_credential_set(&arg_credentials, optarg);
                         if (r < 0)
                                 return r;
                         arg_settings_mask |= SETTING_CREDENTIALS;
@@ -232,7 +232,7 @@ static int parse_argv(int argc, char *argv[]) {
                 }
 
                 case ARG_LOAD_CREDENTIAL: {
-                        r = machine_credential_load(&arg_credentials, &arg_n_credentials, optarg);
+                        r = machine_credential_load(&arg_credentials, optarg);
                         if (r < 0)
                                 return r;
 
@@ -536,10 +536,10 @@ static int run_virtual_machine(void) {
                         return log_oom();
         }
 
-        if (ARCHITECTURE_SUPPORTS_SMBIOS) {
-                ssize_t n;
-                FOREACH_ARRAY(cred, arg_credentials, arg_n_credentials) {
+        if (ARCHITECTURE_SUPPORTS_SMBIOS)
+                FOREACH_ARRAY(cred, arg_credentials.credentials, arg_credentials.n_credentials) {
                         _cleanup_free_ char *cred_data_b64 = NULL;
+                        ssize_t n;
 
                         n = base64mem(cred->data, cred->size, &cred_data_b64);
                         if (n < 0)
@@ -553,7 +553,6 @@ static int run_virtual_machine(void) {
                         if (r < 0)
                                 return log_oom();
                 }
-        }
 
         r = strv_extend(&cmdline, "-drive");
         if (r < 0)
@@ -737,30 +736,21 @@ static int determine_names(void) {
 }
 
 static int run(int argc, char *argv[]) {
-        int r, ret = EXIT_SUCCESS;
+        int r;
 
         log_setup();
 
         r = parse_argv(argc, argv);
         if (r <= 0)
-                goto finish;
+                return r;
 
         r = determine_names();
         if (r < 0)
-                goto finish;
+                return r;
 
         assert_se(sigprocmask_many(SIG_BLOCK, NULL, SIGCHLD, SIGTERM, SIGINT, SIGRTMIN+18, -1) >= 0);
 
-        r = run_virtual_machine();
-        if (r > 0)
-                ret = r;
-finish:
-        machine_credential_free_all(arg_credentials, arg_n_credentials);
-
-        if (r < 0)
-                return r;
-
-        return ret;
+        return run_virtual_machine();
 }
 
 DEFINE_MAIN_FUNCTION_WITH_POSITIVE_FAILURE(run);