At the 'cleanup' label we try to unref @child. However, whenever the label is entered there's no chance for the variable to be anything else than NULL rendering those two lines as dead code. Drop it. And it's the same story with @info. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- libvirt-sandbox/libvirt-sandbox-builder.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/libvirt-sandbox/libvirt-sandbox-builder.c b/libvirt-sandbox/libvirt-sandbox-builder.c index ea7d064..b4b4d77 100644 --- a/libvirt-sandbox/libvirt-sandbox-builder.c +++ b/libvirt-sandbox/libvirt-sandbox-builder.c @@ -752,10 +752,6 @@ gboolean gvir_sandbox_builder_clean_post_stop(GVirSandboxBuilder *builder, ret = FALSE; cleanup: - if (child) - g_object_unref(child); - if (info) - g_object_unref(info); g_object_unref(enumerator); g_object_unref(libsFile); g_free(libsdir); -- 2.4.9

The problem occurs in setup_disk_tags. Imagine that fopen() called at the beginning of the function fails. This results in jumping onto the 'cleanup' label where fclose() is called. However, at this point @fp is NULL. And fclose() does not like that. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- libvirt-sandbox/libvirt-sandbox-init-common.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libvirt-sandbox/libvirt-sandbox-init-common.c b/libvirt-sandbox/libvirt-sandbox-init-common.c index 42beadc..af7e457 100644 --- a/libvirt-sandbox/libvirt-sandbox-init-common.c +++ b/libvirt-sandbox/libvirt-sandbox-init-common.c @@ -109,7 +109,8 @@ static gboolean setup_disk_tags(void) { } ret = TRUE; cleanup: - fclose(fp); + if (fp) + fclose(fp); return ret; } -- 2.4.9

The problem is in gvir_sandbox_config_add_mount_opts. When parsing disk string, "format=" may be within it. This is supposed to change disk format from raw to the desired one. However, due to bug in our implementation, we may end up dereferencing a NULL pointer. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- libvirt-sandbox/libvirt-sandbox-config.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/libvirt-sandbox/libvirt-sandbox-config.c b/libvirt-sandbox/libvirt-sandbox-config.c index 780d174..5a4aacb 100644 --- a/libvirt-sandbox/libvirt-sandbox-config.c +++ b/libvirt-sandbox/libvirt-sandbox-config.c @@ -1611,15 +1611,16 @@ gboolean gvir_sandbox_config_add_mount_opts(GVirSandboxConfig *config, *tmp = '\0'; formatStr = tmp + 1; - if ((strncmp(formatStr, "format=", 7) == 0) && - !(enum_value = g_enum_get_value_by_nick(enum_class, formatStr + 7))) { + if ((strncmp(formatStr, "format=", 7) == 0)) { + if (!(enum_value = g_enum_get_value_by_nick(enum_class, formatStr + 7))) { + g_type_class_unref(enum_class); + g_set_error(error, GVIR_SANDBOX_CONFIG_ERROR, 0, + _("Unknown disk image format: '%s'"), formatStr + 7); + return FALSE; + } g_type_class_unref(enum_class); - g_set_error(error, GVIR_SANDBOX_CONFIG_ERROR, 0, - _("Unknown disk image format: '%s'"), formatStr + 7); - return FALSE; + format = enum_value->value; } - g_type_class_unref(enum_class); - format = enum_value->value; } mnt = GVIR_SANDBOX_CONFIG_MOUNT(g_object_new(type, -- 2.4.9

There's a problem in mount_root(): the return value of fopen() is not checked rather than used directly. Not only this interferes with pattern laid out by other areas of the code, but it's possibly dangerous too. If opening the config file fails, @fp may be dereferenced directly. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- libvirt-sandbox/libvirt-sandbox-init-qemu.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/libvirt-sandbox/libvirt-sandbox-init-qemu.c b/libvirt-sandbox/libvirt-sandbox-init-qemu.c index 054dd67..864db42 100644 --- a/libvirt-sandbox/libvirt-sandbox-init-qemu.c +++ b/libvirt-sandbox/libvirt-sandbox-init-qemu.c @@ -217,6 +217,8 @@ mount_entry(const char *source, } } +#define MOUNTS_CONFIG_FILE SANDBOXCONFIGDIR "/mounts.cfg" + static void mount_root(const char *path) { @@ -226,7 +228,14 @@ mount_root(const char *path) mount_mkdir(SANDBOXCONFIGDIR, 0755); mount_9pfs("sandbox:config", SANDBOXCONFIGDIR, 0755, 1); - FILE *fp = fopen(SANDBOXCONFIGDIR "/mounts.cfg", "r"); + FILE *fp = fopen(MOUNTS_CONFIG_FILE, "r"); + + if (!fp) { + fprintf(stderr, "libvirt-sandbox-init-qemu: %s: can't open %s: %s", + __func__, MOUNTS_CONFIG_FILE, strerror(errno)); + exit_poweroff(); + } + while (fgets(line, sizeof line, fp) && !foundRoot) { char *source = line; char *target = strchr(source, '\t'); -- 2.4.9