10:36 a.m.
This series aims to fix all the issues coverity identified with
libvirt-sandbox. No critical issues found - just dead code, resource
leaks and some missing return checks and a minor crash in CLI tool
parsing.
10:36 a.m.
New subject: [libvirt] [PATCH sandbox 1/9] Check return value from mkdir in libvirt-sandbox-init-qemu
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Most calls to mkdir() in libvirt-sandbox-init-qemu had their
return value checked, but one was missed.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
libvirt-sandbox/libvirt-sandbox-init-qemu.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/libvirt-sandbox/libvirt-sandbox-init-qemu.c
b/libvirt-sandbox/libvirt-sandbox-init-qemu.c
index f09c6d9..012c6a2 100644
--- a/libvirt-sandbox/libvirt-sandbox-init-qemu.c
+++ b/libvirt-sandbox/libvirt-sandbox-init-qemu.c
@@ -541,7 +541,11 @@ print_uptime(void)
static void set_debug(void)
{
- mkdir("/proc", 0755);
+ if (mkdir("/proc", 0755) < 0) {
+ fprintf(stderr, "libvirt-sandbox-init-qemu: %s: cannot mkdir /proc:
%s\n",
+ __func__, strerror(errno));
+ exit_poweroff();
+ }
if (mount("none", "/proc", "proc", 0, "")
< 0) {
fprintf(stderr, "libvirt-sandbox-init-qemu: %s: cannot mount /proc:
%s\n",
__func__, strerror(errno));
--
1.8.3.1
10:36 a.m.
New subject: [libvirt] [PATCH sandbox 2/9] Remove unused 'int fd' variable
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The 'int fd' variable in gvir_sandbox_builder_machine_mkinitrd was
no longer used, causing a coverity warning about dead code.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
libvirt-sandbox/libvirt-sandbox-builder-machine.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/libvirt-sandbox/libvirt-sandbox-builder-machine.c
b/libvirt-sandbox/libvirt-sandbox-builder-machine.c
index 6b9b506..229a6cc 100644
--- a/libvirt-sandbox/libvirt-sandbox-builder-machine.c
+++ b/libvirt-sandbox/libvirt-sandbox-builder-machine.c
@@ -153,7 +153,6 @@ static gchar *gvir_sandbox_builder_machine_mkinitrd(GVirSandboxConfig
*config,
GVirSandboxConfigInitrd *initrd = gvir_sandbox_config_initrd_new();
GVirSandboxBuilderInitrd *builder = gvir_sandbox_builder_initrd_new();
gchar *targetfile = g_strdup_printf("%s/initrd.img", statedir);
- int fd = -1;
gchar *kver = gvir_sandbox_builder_machine_get_kernrelease(config);
const gchar *kmodpath = gvir_sandbox_config_get_kmodpath(config);
if (!kmodpath)
@@ -183,8 +182,6 @@ static gchar *gvir_sandbox_builder_machine_mkinitrd(GVirSandboxConfig
*config,
goto cleanup;
cleanup:
- if (fd != -1)
- close(fd);
if (*error) {
g_free(targetfile);
targetfile = NULL;
--
1.8.3.1
10:36 a.m.
New subject: [libvirt] [PATCH sandbox 3/9] Fix crash if mount option is not fully specified
From: "Daniel P. Berrange" <berrange(a)redhat.com>
If the user specified '-m ram:/tmp' instead of '-m ram:/tmp=500M'
the code would reference a NULL pointer. Fix it to return an
error message instead. This fixes a coverity identified issue.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
libvirt-sandbox/libvirt-sandbox-config.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/libvirt-sandbox/libvirt-sandbox-config.c
b/libvirt-sandbox/libvirt-sandbox-config.c
index 8e8ac65..8eb8d4f 100644
--- a/libvirt-sandbox/libvirt-sandbox-config.c
+++ b/libvirt-sandbox/libvirt-sandbox-config.c
@@ -1224,6 +1224,7 @@ gboolean gvir_sandbox_config_add_mount_strv(GVirSandboxConfig
*config,
* - host-bind:/tmp=/var/lib/sandbox/demo/tmp
* - host-image:/=/var/lib/sandbox/demo.img
* - guest-bind:/home=/tmp/home
+ * - ram:/tmp=500M
*/
gboolean gvir_sandbox_config_add_mount_opts(GVirSandboxConfig *config,
const char *mount,
@@ -1262,13 +1263,16 @@ gboolean gvir_sandbox_config_add_mount_opts(GVirSandboxConfig
*config,
source = tmp + 1;
}
+ if (!tmp) {
+ g_set_error(error, GVIR_SANDBOX_CONFIG_ERROR, 0,
+ _("Missing mount source string on %s"), mount);
+ return FALSE;
+ }
+
if (type == GVIR_SANDBOX_TYPE_CONFIG_MOUNT_RAM) {
gint size;
gchar *end;
- gchar *sizestr;
- *tmp = '\0';
- sizestr = tmp + 1;
- size = strtol(sizestr, &end, 10);
+ size = strtol(source, &end, 10);
if (end) {
if (g_str_equal(end, "KiB") || g_str_equal(end, "K"))
--
1.8.3.1
10:36 a.m.
New subject: [libvirt] [PATCH sandbox 4/9] Add pod docs for 'ram' filesystem mount syntax
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Neither virt-sandbox or virt-sandbox-service man pages documented
the 'ram' filesystem mount syntax. Fix that, and also add new line
breaks in virt-sandbox-service man page.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
bin/virt-sandbox-service-create.pod | 49 +++++++++++++++++++++----------------
bin/virt-sandbox.c | 8 ++++++
2 files changed, 36 insertions(+), 21 deletions(-)
diff --git a/bin/virt-sandbox-service-create.pod b/bin/virt-sandbox-service-create.pod
index fe0226e..2ab289a 100644
--- a/bin/virt-sandbox-service-create.pod
+++ b/bin/virt-sandbox-service-create.pod
@@ -77,41 +77,48 @@ Default: C<UID's Homedir>.
=item B<-m TYPE:DST=SRC>, B<--mount TYPE:DST=SRC>
-Sets up a mount inside the sandbox at B<DST> backed by B<SRC>. The meaning of
B<SRC> depends on the value of "TYPE" specified:
+Sets up a mount inside the sandbox at B<DST> backed by B<SRC>. The
+meaning of B<SRC> depends on the value of C<TYPE> specified:
=over 4
-=item host-bind
+=item B<host-bind>
-If B<TYPE> is B<host-bind>, then B<SRC> is interpreted as the path to a
directory on the host filesystem. If "SRC" is the empty string, then a temporary
(empty) directory is created on the host before starting the sandbox and deleted
afterwards. The "--include" option is useful for populating these temporary
directories with copies of host files.
+If B<TYPE> is B<host-bind>, then B<SRC> is interpreted as the path
+to a directory on the host filesystem. If C<SRC> is the empty string,
+then a temporary (empty) directory is created on the host before
+starting the sandbox and deleted afterwards. The C<--include> option
+is useful for populating these temporary directories with copies of host
+files.
-=back
-
-=over 4
+=item B<host-image>
-=item host-image
+If B<TYPE> is B<host-image>, then B<SRC> is interpreted as the path
+to a disk image file on the host filesystem. The image should be
+formatted with a filesystem that can be auto-detected by the sandbox,
+such as B<ext3>, B<ext4>, etc. The disk image itself should be a raw
+file, not qcow2 or any other special format
-If B<TYPE> is B<host-image>, then B<SRC> is interpreted as the path to
a disk image file on the host filesystem. The image should be formatted with a filesystem
that can be auto-detected by the sandbox, such as B<ext3, ext4>, etc. The disk image
itself should be a raw file, not qcow2 or any other special format
-
-=back
+=item B<guest-bind>
-=over 4
+If B<TYPE> is B<guest-bind>, then B<SRC> is interpreted as the path
+to another directory in the container filesystem.
-=item guest-bind
+=item B<ram>
-If B<TYPE> is B<guest-bind>, then B<SRC> is interpreted as the path to
another directory in the container filesystem.
+If B<TYPE> is B<ram>, then B<SRC> is interpreted as specifying the
+size of the RAM disk in bytes. The suffix B<K>, B<KiB>, B<M>,
+B<MiB>, B<G>, B<GiB> can used to alter the units from bytes to a
+coarser level.
=back
-=over 4
-
-=item Some examples
+Some examples
- -m host-bind:/tmp=/var/lib/sandbox/demo/tmp
- -m host-image:/=/var/lib/sandbox/demo.img
- -m guest-bind:/home=/tmp/home
-
-=back
+ -m host-bind:/tmp=/var/lib/sandbox/demo/tmp
+ -m host-image:/=/var/lib/sandbox/demo.img
+ -m guest-bind:/home=/tmp/home
+ -m ram:/tmp=500M
=item B<-N NETWORK-OPTIONS>, B<--network NETWORK-OPTIONS>
diff --git a/bin/virt-sandbox.c b/bin/virt-sandbox.c
index ae303d2..b16217b 100644
--- a/bin/virt-sandbox.c
+++ b/bin/virt-sandbox.c
@@ -332,6 +332,13 @@ file, not qcow2 or any other special format
If B<TYPE> is B<guest-bind>, then B<SRC> is interpreted as the path
to another directory in the container filesystem.
+=item B<ram>
+
+If B<TYPE> is B<ram>, then B<SRC> is interpreted as specifying the
+size of the RAM disk in bytes. The suffix B<K>, B<KiB>, B<M>,
+B<MiB>, B<G>, B<GiB> can used to alter the units from bytes to a
+coarser level.
+
=back
Some examples
@@ -339,6 +346,7 @@ Some examples
-m host-bind:/tmp=/var/lib/sandbox/demo/tmp
-m host-image:/=/var/lib/sandbox/demo.img
-m guest-bind:/home=/tmp/home
+ -m ram:/tmp=500M
=item B<-I HOST-PATH>, B<--includefile=HOST-PATH>
--
1.8.3.1
10:36 a.m.
New subject: [libvirt] [PATCH sandbox 5/9] Avoid close of un-opened file descriptor
From: "Daniel P. Berrange" <berrange(a)redhat.com>
In libvirt-sandbox-init-qemu if open() returned -1 and errno
was set to EEXIST then close() would be called on a FD that
was -1. This fixes a coverity identified issue.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
libvirt-sandbox/libvirt-sandbox-init-qemu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libvirt-sandbox/libvirt-sandbox-init-qemu.c
b/libvirt-sandbox/libvirt-sandbox-init-qemu.c
index 012c6a2..44f5de0 100644
--- a/libvirt-sandbox/libvirt-sandbox-init-qemu.c
+++ b/libvirt-sandbox/libvirt-sandbox-init-qemu.c
@@ -123,7 +123,8 @@ mount_mkfile(const char *file, int mode)
__func__, file, strerror(errno));
exit_poweroff();
}
- close(fd);
+ if (fd != -1)
+ close(fd);
}
static void
--
1.8.3.1
10:36 a.m.
New subject: [libvirt] [PATCH sandbox 6/9] Fix leak of file handle in libvirt-sandbox-init-common
From: "Daniel P. Berrange" <berrange(a)redhat.com>
If several error cases of the run_interactive method, the
sigpipe or host file descriptors could be leaked.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
libvirt-sandbox/libvirt-sandbox-init-common.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/libvirt-sandbox/libvirt-sandbox-init-common.c
b/libvirt-sandbox/libvirt-sandbox-init-common.c
index c8e8379..262f4e1 100644
--- a/libvirt-sandbox/libvirt-sandbox-init-common.c
+++ b/libvirt-sandbox/libvirt-sandbox-init-common.c
@@ -1076,7 +1076,7 @@ run_interactive(GVirSandboxConfig *config)
if ((host = open(devname, O_RDWR)) < 0) {
g_printerr(_("libvirt-sandbox-init-common: cannot open %s: %s"),
devname, strerror(errno));
- return -1;
+ goto cleanup;
}
tcgetattr(STDIN_FILENO, &rawattr);
@@ -1093,7 +1093,7 @@ run_interactive(GVirSandboxConfig *config)
gvir_sandbox_config_get_userid(config),
gvir_sandbox_config_get_groupid(config),
gvir_sandbox_config_get_homedir(config)) < 0)
- return -1;
+ goto cleanup;
command = gvir_sandbox_config_get_command(config);
if (!eventloop(gvir_sandbox_config_interactive_get_tty(iconfig),
@@ -1112,6 +1112,8 @@ cleanup:
close(sigpipe[0]);
if (sigpipe[1] != -1)
close(sigpipe[1]);
+ if (host != -1)
+ close(host);
return ret;
}
--
1.8.3.1
10:36 a.m.
New subject: [libvirt] [PATCH sandbox 7/9] Fix leak of file handle in libvirt-sandbox-init-qemu
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The 'FILE *' handle used to read /proc/cmdline was not
closed in all codepaths. This caused coverity to identify
a resource leak.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
libvirt-sandbox/libvirt-sandbox-init-qemu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libvirt-sandbox/libvirt-sandbox-init-qemu.c
b/libvirt-sandbox/libvirt-sandbox-init-qemu.c
index 44f5de0..998bd02 100644
--- a/libvirt-sandbox/libvirt-sandbox-init-qemu.c
+++ b/libvirt-sandbox/libvirt-sandbox-init-qemu.c
@@ -557,8 +557,9 @@ static void set_debug(void)
if (fp && fgets(line, sizeof line, fp)) {
if (strstr(line, "debug"))
debug=1;
- fclose(fp);
}
+ if (fp)
+ fclose(fp);
if (umount("/proc") < 0) {
fprintf(stderr, "libvirt-sandbox-init-qemu: %s: cannot unmount /proc:
%s\n",
__func__, strerror(errno));
--
1.8.3.1
10:36 a.m.
New subject: [libvirt] [PATCH sandbox 8/9] Remove bogus check for NULL in cleanup path
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The 'config' object in libvirt-sandbox-context-service.c
methods cannot be NULL, so checking 'if (config)' is
pointless code.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
libvirt-sandbox/libvirt-sandbox-context-service.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/libvirt-sandbox/libvirt-sandbox-context-service.c
b/libvirt-sandbox/libvirt-sandbox-context-service.c
index 0858ffb..b1ee311 100644
--- a/libvirt-sandbox/libvirt-sandbox-context-service.c
+++ b/libvirt-sandbox/libvirt-sandbox-context-service.c
@@ -179,8 +179,7 @@ cleanup:
g_object_unref(builder);
if (connection)
g_object_unref(connection);
- if (config)
- g_object_unref(config);
+ g_object_unref(config);
return ret;
}
@@ -265,8 +264,7 @@ cleanup:
g_object_unref(builder);
if (connection)
g_object_unref(connection);
- if (config)
- g_object_unref(config);
+ g_object_unref(config);
return ret;
}
--
1.8.3.1
10:36 a.m.
New subject: [libvirt] [PATCH sandbox 9/9] Fix broken 'default' case in switch statement
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The 'default' case somehow got placed on the wrong line,
leading to unreachable code.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
libvirt-sandbox/libvirt-sandbox-config-interactive.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/libvirt-sandbox/libvirt-sandbox-config-interactive.c
b/libvirt-sandbox/libvirt-sandbox-config-interactive.c
index 6d0e1e8..6fbb1c6 100644
--- a/libvirt-sandbox/libvirt-sandbox-config-interactive.c
+++ b/libvirt-sandbox/libvirt-sandbox-config-interactive.c
@@ -91,12 +91,11 @@ static void gvir_sandbox_config_interactive_set_property(GObject
*object,
GVirSandboxConfigInteractivePrivate *priv = config->priv;
switch (prop_id) {
-
- default:
case PROP_TTY:
priv->tty = g_value_get_boolean(value);
break;
+ default:
G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
}
}
--
1.8.3.1
5:21 a.m.
On Thu, Aug 15, 2013 at 04:36:38PM +0100, Daniel P. Berrange wrote:
This series aims to fix all the issues coverity identified with
libvirt-sandbox. No critical issues found - just dead code, resource
leaks and some missing return checks and a minor crash in CLI tool
parsing.
This series is now applied to master
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4113
days inactive
4117
days old
10 comments
1 participants
participants (1)
-
Daniel P. Berrange