vmspawn: terminate the child processes before removing the runtime directory
The runtime directory holds what the helpers we fork off need: swtpm's socket and, unless --tpm-state= points elsewhere, its state, the virtiofsd sockets, the varlink control socket, and the QEMU config file. runtime_dir was declared after the CLEANUP_ARRAY() that terminates those helpers, and cleanup handlers run in reverse declaration order, so the directory was removed while all of them were still running. That produces spurious errors from the helpers, and anything they create while rm_rf() walks the tree can make the final rmdir fail and leave the runtime directory behind. Declare runtime_dir before the CLEANUP_ARRAY() instead, so the helpers are sent SIGTERM and reaped first and the directory only goes away once nothing writes to it anymore. While at it, drop the rm_rf_physical_and_freep() destructors from the SSH key paths. They are only ever assigned paths below the runtime directory, whose removal takes them along, so those destructors never had an effect.
This commit is contained in:
committed by
Yu Watanabe
parent
844c44a0d4
commit
57c4a1ea32
@@ -2559,7 +2559,8 @@ static int discover_ovmf_config(OvmfConfig **ret, sd_json_variant **ret_firmware
|
||||
static int run_virtual_machine(int kvm_device_fd, int vhost_device_fd) {
|
||||
_cleanup_(ovmf_config_freep) OvmfConfig *ovmf_config = NULL;
|
||||
_cleanup_free_ char *qemu_binary = NULL, *mem = NULL;
|
||||
_cleanup_(rm_rf_physical_and_freep) char *ssh_private_key_path = NULL, *ssh_public_key_path = NULL;
|
||||
/* Always assigned paths below the runtime directory, whose removal takes them along. */
|
||||
_cleanup_free_ char *ssh_private_key_path = NULL, *ssh_public_key_path = NULL;
|
||||
_cleanup_(rm_rf_subvolume_and_freep) char *snapshot_directory = NULL;
|
||||
_cleanup_(release_lock_file) LockFile tree_global_lock = LOCK_FILE_INIT, tree_local_lock = LOCK_FILE_INIT;
|
||||
_cleanup_close_ int notify_sock_fd = -EBADF;
|
||||
@@ -2569,6 +2570,11 @@ static int run_virtual_machine(int kvm_device_fd, int vhost_device_fd) {
|
||||
.network = { .fd = -EBADF },
|
||||
.vsock = { .fd = -EBADF },
|
||||
};
|
||||
/* Declared before the CLEANUP_ARRAY() below, so that cleanup order (reverse of declaration) lets
|
||||
* fork_notify_terminate_many() reap the helpers before this directory goes away: their sockets,
|
||||
* state, and the QEMU config file all live below it, and pulling it out from under them gives
|
||||
* spurious errors and can leave the directory behind. */
|
||||
_cleanup_(rm_rf_physical_and_freep) char *runtime_dir = NULL;
|
||||
sd_event_source **children = NULL;
|
||||
size_t n_children = 0, n_pass_fds = 0;
|
||||
int r;
|
||||
@@ -2698,8 +2704,6 @@ static int run_virtual_machine(int kvm_device_fd, int vhost_device_fd) {
|
||||
|
||||
/* Create our runtime directory. We need this for the QMP varlink control socket, the QEMU
|
||||
* config file, TPM state, virtiofsd sockets, runtime mounts, and SSH key material. */
|
||||
_cleanup_(rm_rf_physical_and_freep) char *runtime_dir = NULL;
|
||||
|
||||
r = runtime_directory_make(arg_runtime_scope, "systemd/vmspawn", arg_machine, &runtime_dir);
|
||||
if (r < 0)
|
||||
return log_error_errno(r, "Failed to create runtime directory: %m");
|
||||
|
||||
Reference in New Issue
Block a user