On Mon, 2021-03-01 at 12:49 +0100, Michal Privoznik wrote:
> This problem is reproducible only with secret driver. When
> starting a domain via virt-qemu-run and both secret and
> (nonexistent) root directory specified this is what happens:
>
> 1) virt-qemu-run opens "secret:///embed?root=$rootdir"
> connection, which results in the secret driver initialization
> (done in secretStateInitialize()). During this process, the
> driver creates it's own configDir (derived from $rootdir)
s/it's own/its own/
> including those parents which don't exists yet. This is all
> done with the mode S_IRWXU and thus results in the $rootdir
> being created with very restrictive mode (specifically, +x is
> missing for group and others).
>
> 2) now, virt-qemu-run-opens "qemu:///embed?root=$rootdir" and
s/run-opens/run opens/
> +++ b/src/qemu/qemu_shim.c
> @@ -213,11 +213,16 @@ int main(int argc, char **argv)
> }
> tmproot = true;
>
> - if (chmod(root, 0755) < 0) {
> - g_printerr("%s: cannot chown temporary dir: %s\n",
> - argv[0], g_strerror(errno));
> - goto cleanup;
> - }
> + } else if (g_mkdir_with_parents(root, 0755) < 0) {
> + g_printerr("%s: cannot create dir: %s\n",
> + argv[0], g_strerror(errno));
> + goto cleanup;
> + }
> +
> + if (chmod(root, 0755) < 0) {
> + g_printerr("%s: cannot chmod temporary dir: %s\n",
> + argv[0], g_strerror(errno));
> + goto cleanup;
> }
Wouldn't it make sense to leave the chmod() bit where it was?
g_mkdir_with_parents() already accepts the mode as a parameter, so
calling chmod() again seems unnecessary.
With that changed and the commit message fixed,
Reviewed-by: Andrea Bolognani <abologna@xxxxxxxxxx>
--
Andrea Bolognani / Red Hat / Virtualization
