[libvirt] [PATCH] Fix double-free and broken logic in virt-login-shell
by Daniel P. Berrange
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The virLoginShellAllowedUser method must not free the 'groups'
parameter it is given, as that is owned by the caller.
The virLoginShellAllowedUser method should be checking
'!*ptr' (ie empty string) rather than '!ptr' (NULL string)
since the latter cannot be true.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
tools/virt-login-shell.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/tools/virt-login-shell.c b/tools/virt-login-shell.c
index b8f1a28..b27e44f 100644
--- a/tools/virt-login-shell.c
+++ b/tools/virt-login-shell.c
@@ -85,7 +85,7 @@ static int virLoginShellAllowedUser(virConfPtr conf,
*/
if (pp->str[0] == '%') {
ptr = &pp->str[1];
- if (!ptr)
+ if (!*ptr)
continue;
for (i = 0; groups[i]; i++) {
if (!(gname = virGetGroupName(groups[i])))
@@ -96,7 +96,6 @@ static int virLoginShellAllowedUser(virConfPtr conf,
}
VIR_FREE(gname);
}
- VIR_FREE(groups);
continue;
}
if (fnmatch(pp->str, name, 0) == 0) {
@@ -109,7 +108,6 @@ static int virLoginShellAllowedUser(virConfPtr conf,
virReportSystemError(EPERM, _("%s not listed as an allowed_users in %s"), name, conf_file);
cleanup:
VIR_FREE(gname);
- VIR_FREE(groups);
return ret;
}
--
1.8.3.1
11 years, 3 months
[libvirt] Updated patch for virt-login-shell for joing libvirt lxc containers.
by Daniel J Walsh
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
THis patch fixes all of Eric's and Daniels comments.
[PATCH] virt-login-shell joins users into lxc container.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.14 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iEYEARECAAYFAlH7zp8ACgkQrlYvE4MpobNx3gCbBtxw7T4fzIfHSyEEKKyjojXR
BUUAoOToptiTOi+RC6Bdcp+zvg/xzfRh
=7zpw
-----END PGP SIGNATURE-----
11 years, 3 months
[libvirt] [PATCH] virnettlscontext: Resolve Coverity warnings (UNINIT)
by John Ferlan
Coverity complained about the usage of the uninitialized cacerts in the
event(s) that "access(certFile, R_OK)" and/or "access(cacertFile, R_OK)"
fail the for loop used to fill in the certs will have indeterminate data
as well as the possibility that both failures would result in the
gnutls_x509_crt_deinit() call having a similar fate.
Initializing cacerts only would resolve the issue; however, it still
would leave the indeterminate action, so rather add a parameter to
the virNetTLSContextLoadCACertListFromFile() to pass the max size rather
then overloading the returned count parameter. If the the call is never
made, then we won't go through the for loops referencing the empty
cacerts
---
src/rpc/virnettlscontext.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
index 2beee8f..7cee27c 100644
--- a/src/rpc/virnettlscontext.c
+++ b/src/rpc/virnettlscontext.c
@@ -545,12 +545,12 @@ cleanup:
static int virNetTLSContextLoadCACertListFromFile(const char *certFile,
gnutls_x509_crt_t *certs,
+ unsigned int certMax,
size_t *ncerts)
{
gnutls_datum_t data;
char *buf = NULL;
int ret = -1;
- unsigned int certMax = *ncerts;
*ncerts = 0;
VIR_DEBUG("certFile %s", certFile);
@@ -584,15 +584,17 @@ static int virNetTLSContextSanityCheckCredentials(bool isServer,
{
gnutls_x509_crt_t cert = NULL;
gnutls_x509_crt_t cacerts[MAX_CERTS];
- size_t ncacerts = MAX_CERTS;
+ size_t ncacerts = 0;
size_t i;
int ret = -1;
+ memset(cacerts, 0, sizeof(cacerts));
if ((access(certFile, R_OK) == 0) &&
!(cert = virNetTLSContextLoadCertFromFile(certFile, isServer)))
goto cleanup;
if ((access(cacertFile, R_OK) == 0) &&
- virNetTLSContextLoadCACertListFromFile(cacertFile, cacerts, &ncacerts) < 0)
+ virNetTLSContextLoadCACertListFromFile(cacertFile, cacerts,
+ MAX_CERTS, &ncacerts) < 0)
goto cleanup;
if (cert &&
--
1.8.3.1
11 years, 3 months
[libvirt] [libvirt-sandbox][PATCH v2] Fix logical judgement in get_name
by Alex Jia
As usual, this issue can't be hit, but from codes point of view,
if deliberately remove 'name' in the configuration, and then the
'Name not congfigured' error message can't be raised unless the
configuration file doesn't exist, in fact, the get_name() will
directly return None without expected error.
Signed-off-by: Alex Jia <ajia(a)redhat.com>
---
bin/virt-sandbox-service | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service
index 03873c9..26b4a40 100755
--- a/bin/virt-sandbox-service
+++ b/bin/virt-sandbox-service
@@ -453,8 +453,12 @@ WantedBy=multi-user.target
def get_name(self):
if self.config:
- return self.config.get_name()
- raise ValueError([_("Name not configured")])
+ name = self.config.get_name()
+ if not name:
+ raise ValueError([_("Name not configured")])
+ return name
+ sys.stderr.write("The configuration %s does not exist\n" % self.config)
+ sys.exit(1)
def set_copy(self, copy):
self.copy = copy
--
1.8.3.1
11 years, 3 months
[libvirt] LXC: Helper function for checking ownership of dir when userns enabled
by Chen Hanxiao
From: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com>
If we enable userns, the ownership of dir we provided for containers
should match the uid/gid in idmap.
Currently, the debug log is very implicit or misleading sometimes.
This patch will help clarify this for us when using
debug log or virsh.
Signed-off-by: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com>
---
src/lxc/lxc_container.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 45 insertions(+), 0 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index b910b10..ce17466 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -1815,6 +1815,48 @@ lxcNeedNetworkNamespace(virDomainDefPtr def)
return false;
}
+/*
+ * Helper function for helping check
+ * whether we have enough privilege
+ * to operate the source dir when userns enabled
+ * @vmDef: pointer to vm definition structure
+ * Returns 0 on success or -1 in case of error
+ */
+static int
+lxcContainerUsernsSrcOwnershipCheck(virDomainDefPtr vmDef)
+{
+ struct stat buf;
+ int i;
+ uid_t uid;
+ gid_t gid;
+
+ for(i=0; i < vmDef->nfss; i++) {
+ VIR_DEBUG("dst is %s, src is %s",
+ vmDef->fss[i]->dst,
+ vmDef->fss[i]->src);
+
+ uid = vmDef->idmap.uidmap[0].target;
+ gid = vmDef->idmap.gidmap[0].target;
+
+ if (lstat(vmDef->fss[i]->src, &buf) < 0) {
+ virReportSystemError(errno, _("Cannot access '%s'"),
+ vmDef->fss[i]->src);
+ return -1;
+ } else if(uid != buf.st_uid || gid != buf.st_gid) {
+ VIR_DEBUG("In userns uid is %d, gid is %d\n",
+ uid, gid);
+ errno = EINVAL;
+
+ virReportSystemError(errno,
+ "[userns] Src dir \"%s\" does not belong to uid/gid:%d/%d",
+ vmDef->fss[i]->src, uid, gid);
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
/**
* lxcContainerStart:
* @def: pointer to virtual machine structure
@@ -1866,6 +1908,9 @@ int lxcContainerStart(virDomainDefPtr def,
if (userns_supported()) {
VIR_DEBUG("Enable user namespace");
cflags |= CLONE_NEWUSER;
+ if(lxcContainerUsernsSrcOwnershipCheck(def) < 0) {
+ return -1;
+ }
} else {
virReportSystemError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("Kernel doesn't support user namespace"));
--
1.7.1
11 years, 3 months
[libvirt] [PATCH] Add a man page for virtlockd daemon
by Daniel P. Berrange
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Create a virtlockd.pod.in file containing the man page
content for virtlockd.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
.gitignore | 2 +
libvirt.spec.in | 1 +
src/Makefile.am | 24 ++++++-
src/locking/virtlockd.pod.in | 158 +++++++++++++++++++++++++++++++++++++++++++
4 files changed, 184 insertions(+), 1 deletion(-)
create mode 100644 src/locking/virtlockd.pod.in
diff --git a/.gitignore b/.gitignore
index ae9de0b..26bd829 100644
--- a/.gitignore
+++ b/.gitignore
@@ -144,6 +144,8 @@
/src/util/virkeymaps.h
/src/virt-aa-helper
/src/virtlockd
+/src/virtlockd.8
+/src/virtlockd.8.in
/src/virtlockd.init
/tests/*.log
/tests/*.pid
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 79c5a2c..a95b783 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1872,6 +1872,7 @@ fi
%attr(0755, root, root) %{_sbindir}/virtlockd
%{_mandir}/man8/libvirtd.8*
+%{_mandir}/man8/virtlockd.8*
%if %{with_driver_modules}
%if %{with_network}
diff --git a/src/Makefile.am b/src/Makefile.am
index 277f749..d351539 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -2019,9 +2019,31 @@ virtlockd.init: locking/virtlockd.init.in $(top_builddir)/config.status
chmod a+x $@-t && \
mv $@-t $@
+POD2MAN = pod2man -c "Virtualization Support" \
+ -r "$(PACKAGE)-$(VERSION)" -s 8
+$(srcdir)/virtlockd.8.in: locking/virtlockd.pod.in $(top_srcdir)/configure.ac
+ $(AM_V_GEN)$(POD2MAN) --name VIRTLOCKD $< $@ \
+ && if grep 'POD ERROR' $@ ; then rm $@; exit 1; fi
+
+virtlockd.8: $(srcdir)/virtlockd.8.in
+ $(AM_V_GEN)sed \
+ -e 's|[@]sysconfdir[@]|$(sysconfdir)|g' \
+ -e 's|[@]localstatedir[@]|$(localstatedir)|g' \
+ < $< > $@-t && \
+ mv $@-t $@
+
+man8_MANS = virtlockd.8
+
+MAINTAINERCLEANFILES += $(srcdir)/virtlockd.8.in
+
+EXTRA_DIST += \
+ locking/virtlockd.service.in \
+ locking/virtlockd.socket.in \
+ locking/virtlockd.pod.in \
+ virtlockd.8.in \
+ $(NULL)
-EXTRA_DIST += locking/virtlockd.service.in locking/virtlockd.socket.in
if WITH_LIBVIRTD
if LIBVIRT_INIT_SCRIPT_SYSTEMD
diff --git a/src/locking/virtlockd.pod.in b/src/locking/virtlockd.pod.in
new file mode 100644
index 0000000..2621d2c
--- /dev/null
+++ b/src/locking/virtlockd.pod.in
@@ -0,0 +1,158 @@
+=head1 NAME
+
+virtlockd - libvirt lock management daemon
+
+=head1 SYNOPSIS
+
+B<virtlockd> [ -dv ] [ -f config_file ] [ -p pid_file ]
+
+B<virtlockd> --version
+
+=head1 DESCRIPTION
+
+The B<virtlockd> program is a server side daemon component of the libvirt
+virtualization management system that is used to manage locks held against
+virtual machine resources, such as their disks.
+
+This daemon is not used directly by libvirt client applications, rather it
+is called on their behalf by B<libvirtd>. By maintaining the locks in a
+standalone daemon, the main libvirtd daemon can be restarted without risk
+of loosing locks. The B<virtlockd> daemon has the ability to re-exec()
+itself upon receiving SIGUSR1, to allow live upgrades without downtime.
+
+The virtlockd daemon listens for requests on a local Unix domain socket.
+
+=head1 OPTIONS
+
+=over
+
+=item B<-d, --daemon>
+
+Run as a daemon & write PID file.
+
+=item B<-f, --config> I<FILE>
+
+Use this configuration file, overriding the default value.
+
+=item B<-p, --pid-file> I<FILE>
+
+Use this name for the PID file, overriding the default value.
+
+=item B<-v, --verbose>
+
+Enable output of verbose messages.
+
+=item B< --version>
+
+Display version information then exit.
+
+=back
+
+=head1 SIGNALS
+
+On receipt of B<SIGUSR1> virtlockd will re-exec() its binary, while
+maintaining all current locks and clients. This allows for live
+upgrades of the virtlockd service.
+
+=head1 FILES
+
+=head2 When run as B<root>.
+
+=over
+
+=item F<SYSCONFDIR/virtlockd.conf>
+
+The default configuration file used by virtlockd, unless overridden on the
+command line using the B<-f>|B<--config> option.
+
+=item F<LOCALSTATEDIR/run/libvirt/virtlockd-sock>
+
+The sockets libvirtd will use.
+
+=item F<LOCALSTATEDIR/run/virtlockd.pid>
+
+The PID file to use, unless overridden by the B<-p>|B<--pid-file> option.
+
+=back
+
+=head2 When run as B<non-root>.
+
+=over
+
+=item F<$XDG_CONFIG_HOME/virtlockd.conf>
+
+The default configuration file used by libvirtd, unless overridden on the
+command line using the B<-f>|B<--config> option.
+
+=item F<$XDG_RUNTIME_DIR/libvirt/virtlockd-sock>
+
+The socket libvirtd will use.
+
+=item F<$XDG_RUNTIME_DIR/libvirt/virtlockd.pid>
+
+The PID file to use, unless overridden by the B<-p>|B<--pid-file> option.
+
+=item If $XDG_CONFIG_HOME is not set in your environment, libvirtd will use F<$HOME/.config>
+
+=item If $XDG_RUNTIME_DIR is not set in your environment, libvirtd will use F<$HOME/.cache>
+
+=back
+
+=head1 EXAMPLES
+
+To retrieve the version of virtlockd:
+
+ # virtlockd --version
+ virtlockd (libvirt) 1.1.1
+ #
+
+To start virtlockd, instructing it to daemonize and create a PID file:
+
+ # virtlockd -d
+ # ls -la LOCALSTATEDIR/run/virtlockd.pid
+ -rw-r--r-- 1 root root 6 Jul 9 02:40 LOCALSTATEDIR/run/virtlockd.pid
+ #
+
+=head1 BUGS
+
+Please report all bugs you discover. This should be done via either:
+
+=over
+
+=item a) the mailing list
+
+L<http://libvirt.org/contact.html>
+
+=item or,
+
+B<>
+
+=item b) the bug tracker
+
+L<http://libvirt.org/bugs.html>
+
+=item Alternatively, you may report bugs to your software distributor / vendor.
+
+=back
+
+=head1 AUTHORS
+
+Please refer to the AUTHORS file distributed with libvirt.
+
+=head1 COPYRIGHT
+
+Copyright (C) 2006-2013 Red Hat, Inc., and the authors listed in the
+libvirt AUTHORS file.
+
+=head1 LICENSE
+
+virtlockd is distributed under the terms of the GNU LGPL v2.1+.
+This is free software; see the source for copying conditions. There
+is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
+PURPOSE
+
+=head1 SEE ALSO
+
+L<libvirtd(8)>, L<http://www.libvirt.org/>
+
+=cut
--
1.8.3.1
11 years, 3 months
[libvirt] [PATCH] tests: work with older dbus
by Eric Blake
On RHEL 5, with dbus 1.1.2, compilation failed with:
virsystemdmock.c: In function 'dbus_connection_send_with_reply_and_block':
virsystemdmock.c:68: warning: implicit declaration of function 'dbus_message_set_serial'
Fix this by instead bypassing all attempts to use a dbus serial.
* tests/virsystemdmock.c (dbus_message_set_reply_serial): Add new
override.
(dbus_connection_send_with_reply_and_block): No longer bother with
the serial.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
I'm tempted to push this under the build-breaker rule, but it is
still pending a review of this patch also for RHEL 5:
https://www.redhat.com/archives/libvir-list/2013-August/msg00313.html
tests/virsystemdmock.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tests/virsystemdmock.c b/tests/virsystemdmock.c
index b6c3695..ded52d2 100644
--- a/tests/virsystemdmock.c
+++ b/tests/virsystemdmock.c
@@ -58,6 +58,12 @@ dbus_bool_t dbus_connection_set_watch_functions(DBusConnection *connection ATTRI
return 1;
}
+dbus_bool_t dbus_message_set_reply_serial(DBusMessage *message ATTRIBUTE_UNUSED,
+ dbus_uint32_t serial ATTRIBUTE_UNUSED)
+{
+ return 1;
+}
+
DBusMessage *dbus_connection_send_with_reply_and_block(DBusConnection *connection ATTRIBUTE_UNUSED,
DBusMessage *message,
int timeout_milliseconds ATTRIBUTE_UNUSED,
@@ -65,8 +71,6 @@ DBusMessage *dbus_connection_send_with_reply_and_block(DBusConnection *connectio
{
DBusMessage *reply = NULL;
- dbus_message_set_serial(message, 7);
-
if (getenv("FAIL_BAD_SERVICE"))
reply = dbus_message_new_error(message,
"org.freedesktop.systemd.badthing",
--
1.8.3.1
11 years, 3 months
[libvirt] [PATCH] build: fix compilation of virt-login-shell.c
by Jim Fehlig
virt-login-shell.c was failing to compile with
CC virt_login_shell-virt-login-shell.o
virt-login-shell.c: In function 'main':
virt-login-shell.c:205:5: error: implicit declaration of function 'setlocale' [-Werror=implicit-function-declaration]
virt-login-shell.c:205:5: error: nested extern declaration of 'setlocale' [-Werror=nested-externs]
virt-login-shell.c:205:20: error: 'LC_ALL' undeclared (first use in this function)
---
I'm a little surprised that others are not seeing this problem, so a bit
nervious committing without an ACK.
tools/virt-login-shell.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/virt-login-shell.c b/tools/virt-login-shell.c
index ffbc713..b8f1a28 100644
--- a/tools/virt-login-shell.c
+++ b/tools/virt-login-shell.c
@@ -27,6 +27,7 @@
#include <errno.h>
#include <stdlib.h>
#include <fnmatch.h>
+#include <locale.h>
#include "internal.h"
#include "virerror.h"
--
1.8.1.4
11 years, 3 months