10:14 p.m.
I've completed and pushed my backport work for both CVEs that
were patched today, into all branches v0.10.2-maint and newer
(basically, we have 0.10.2, then all releases since 1.0.2). One
last branch in active use (hello Debian) remains to be patched;
but here, the backport work had enough conflict resolutions that
I decided to post my work for review first.
Daniel P. Berrange (9):
Introduce APIs for splitting/joining strings
Rename virKillProcess to virProcessKill
Rename virPid{Abort, Wait} to virProcess{Abort, Wait}
Rename virCommandTranslateStatus to virProcessTranslateStatus
Move virProcessKill into virprocess.{h, c}
Move virProcess{Kill, Abort, TranslateStatus} into virprocess.{c, h}
Include process start time when doing polkit checks
Add support for using 3-arg pkcheck syntax for process (CVE-2013-4311)
Fix crash in remoteDispatchDomainMemoryStats (CVE-2013-4296)
.gitignore | 1 +
configure.ac | 8 +
daemon/libvirtd.c | 3 +-
daemon/remote.c | 33 +++-
libvirt.spec.in | 3 +-
po/POTFILES.in | 1 +
src/Makefile.am | 2 +
src/libvirt_private.syms | 16 +-
src/lxc/lxc_container.c | 3 +-
src/lxc/lxc_controller.c | 3 +-
src/qemu/qemu_agent.c | 3 +-
src/qemu/qemu_monitor.c | 3 +-
src/qemu/qemu_process.c | 3 +-
src/rpc/virnetserverclient.c | 8 +-
src/rpc/virnetserverclient.h | 3 +-
src/rpc/virnetsocket.c | 22 ++-
src/rpc/virnetsocket.h | 3 +-
src/uml/uml_driver.c | 3 +-
src/util/command.c | 167 ++------------------
src/util/command.h | 8 -
src/util/util.c | 64 +-------
src/util/util.h | 1 -
src/util/virprocess.c | 359 +++++++++++++++++++++++++++++++++++++++++++
src/util/virprocess.h | 45 ++++++
src/util/virstring.c | 179 +++++++++++++++++++++
src/util/virstring.h | 40 +++++
tests/Makefile.am | 9 +-
tests/testutils.c | 5 +-
tests/virstringtest.c | 161 +++++++++++++++++++
29 files changed, 908 insertions(+), 251 deletions(-)
create mode 100644 src/util/virprocess.c
create mode 100644 src/util/virprocess.h
create mode 100644 src/util/virstring.c
create mode 100644 src/util/virstring.h
create mode 100644 tests/virstringtest.c
--
1.8.3.1
10:14 p.m.
New subject: [libvirt] [v0.9.12-maint 1/9] Introduce APIs for splitting/joining strings
From: "Daniel P. Berrange" <berrange(a)redhat.com>
This introduces a few new APIs for dealing with strings.
One to split a char * into a char **, another to join a
char ** into a char *, and finally one to free a char **
There is a simple test suite to validate the edge cases
too. No more need to use the horrible strtok_r() API,
or hand-written code for splitting strings.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
(cherry picked from commit 76c1fd33c8093d6a7173a85486e1e6f51a832135)
Signed-off-by: Eric Blake <eblake(a)redhat.com>
Conflicts:
tests/Makefile.am - several intermediate tests not backported
---
.gitignore | 1 +
src/Makefile.am | 1 +
src/libvirt_private.syms | 6 ++
src/util/virstring.c | 168 +++++++++++++++++++++++++++++++++++++++++++++++
src/util/virstring.h | 38 +++++++++++
tests/Makefile.am | 9 ++-
tests/virstringtest.c | 161 +++++++++++++++++++++++++++++++++++++++++++++
7 files changed, 383 insertions(+), 1 deletion(-)
create mode 100644 src/util/virstring.c
create mode 100644 src/util/virstring.h
create mode 100644 tests/virstringtest.c
diff --git a/.gitignore b/.gitignore
index 3cc30e8..cc52789 100644
--- a/.gitignore
+++ b/.gitignore
@@ -147,6 +147,7 @@
/tests/virkeyfiletest
/tests/virnet*test
/tests/virshtest
+/tests/virstringtest
/tests/virtimetest
/tests/viruritest
/tests/vmx2xmltest
diff --git a/src/Makefile.am b/src/Makefile.am
index 0dadc29..b5da2fb 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -106,6 +106,7 @@ UTIL_SOURCES = \
util/virnetlink.c util/virnetlink.h \
util/virrandom.h util/virrandom.c \
util/virsocketaddr.h util/virsocketaddr.c \
+ util/virstring.h util/virstring.c \
util/virtime.h util/virtime.c \
util/viruri.h util/viruri.c
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index afb308d..8328fcf 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1490,6 +1490,12 @@ virSetErrorLogPriorityFunc;
virStrerror;
+# virstring.h
+virStringSplit;
+virStringJoin;
+virStringFreeList;
+
+
# virtime.h
virTimeFieldsNow;
virTimeFieldsNowRaw;
diff --git a/src/util/virstring.c b/src/util/virstring.c
new file mode 100644
index 0000000..1917e9a
--- /dev/null
+++ b/src/util/virstring.c
@@ -0,0 +1,168 @@
+/*
+ * Copyright (C) 2012 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <http://www.gnu.org/licenses/>;.
+ *
+ * Authors:
+ * Daniel P. Berrange <berrange(a)redhat.com>
+ */
+
+#include <config.h>
+
+#include "virstring.h"
+#include "memory.h"
+#include "buf.h"
+#include "virterror_internal.h"
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+/*
+ * The following virStringSplit & virStringJoin methods
+ * are derived from g_strsplit / g_strjoin in glib2,
+ * also available under the LGPLv2+ license terms
+ */
+
+/**
+ * virStringSplit:
+ * @string: a string to split
+ * @delim: a string which specifies the places at which to split
+ * the string. The delimiter is not included in any of the resulting
+ * strings, unless @max_tokens is reached.
+ * @max_tokens: the maximum number of pieces to split @string into.
+ * If this is 0, the string is split completely.
+ *
+ * Splits a string into a maximum of @max_tokens pieces, using the given
+ * @delim. If @max_tokens is reached, the remainder of @string is
+ * appended to the last token.
+ *
+ * As a special case, the result of splitting the empty string "" is an empty
+ * vector, not a vector containing a single string. The reason for this
+ * special case is that being able to represent a empty vector is typically
+ * more useful than consistent handling of empty elements. If you do need
+ * to represent empty elements, you'll need to check for the empty string
+ * before calling virStringSplit().
+ *
+ * Return value: a newly-allocated NULL-terminated array of strings. Use
+ * virStringFreeList() to free it.
+ */
+char **virStringSplit(const char *string,
+ const char *delim,
+ size_t max_tokens)
+{
+ char **tokens = NULL;
+ size_t ntokens = 0;
+ size_t maxtokens = 0;
+ const char *remainder = string;
+ char *tmp;
+ size_t i;
+
+ if (max_tokens == 0)
+ max_tokens = INT_MAX;
+
+ tmp = strstr(remainder, delim);
+ if (tmp) {
+ size_t delimlen = strlen(delim);
+
+ while (--max_tokens && tmp) {
+ size_t len = tmp - remainder;
+
+ if (VIR_RESIZE_N(tokens, maxtokens, ntokens, 1) < 0)
+ goto no_memory;
+
+ if (!(tokens[ntokens] = strndup(remainder, len)))
+ goto no_memory;
+ ntokens++;
+ remainder = tmp + delimlen;
+ tmp = strstr(remainder, delim);
+ }
+ }
+ if (*string) {
+ if (VIR_RESIZE_N(tokens, maxtokens, ntokens, 1) < 0)
+ goto no_memory;
+
+ if (!(tokens[ntokens] = strdup(remainder)))
+ goto no_memory;
+ ntokens++;
+ }
+
+ if (VIR_RESIZE_N(tokens, maxtokens, ntokens, 1) < 0)
+ goto no_memory;
+ tokens[ntokens++] = NULL;
+
+ return tokens;
+
+no_memory:
+ virReportOOMError();
+ for (i = 0 ; i < ntokens ; i++)
+ VIR_FREE(tokens[i]);
+ VIR_FREE(tokens);
+ return NULL;
+}
+
+
+/**
+ * virStringJoin:
+ * @strings: a NULL-terminated array of strings to join
+ * @delim: a string to insert between each of the strings
+ *
+ * Joins a number of strings together to form one long string, with the
+ * @delim inserted between each of them. The returned string
+ * should be freed with VIR_FREE().
+ *
+ * Returns: a newly-allocated string containing all of the strings joined
+ * together, with @delim between them
+ */
+char *virStringJoin(const char **strings,
+ const char *delim)
+{
+ char *ret;
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ while (*strings) {
+ virBufferAdd(&buf, *strings, -1);
+ if (*(strings+1))
+ virBufferAdd(&buf, delim, -1);
+ strings++;
+ }
+ if (virBufferError(&buf)) {
+ virReportOOMError();
+ return NULL;
+ }
+ ret = virBufferContentAndReset(&buf);
+ if (!ret) {
+ if (!(ret = strdup(""))) {
+ virReportOOMError();
+ return NULL;
+ }
+ }
+ return ret;
+}
+
+
+/**
+ * virStringFreeList:
+ * @str_array: a NULL-terminated array of strings to free
+ *
+ * Frees a NULL-terminated array of strings, and the array itself.
+ * If called on a NULL value, virStringFreeList() simply returns.
+ */
+void virStringFreeList(char **strings)
+{
+ char **tmp = strings;
+ while (tmp && *tmp) {
+ VIR_FREE(*tmp);
+ tmp++;
+ }
+ VIR_FREE(strings);
+}
diff --git a/src/util/virstring.h b/src/util/virstring.h
new file mode 100644
index 0000000..a569fe0
--- /dev/null
+++ b/src/util/virstring.h
@@ -0,0 +1,38 @@
+/*
+ * Copyright (C) 2007-2012 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <http://www.gnu.org/licenses/>;.
+ *
+ * Authors:
+ * Daniel P. Berrange <berrange(a)redhat.com>
+ */
+
+#ifndef __VIR_STRING_H__
+# define __VIR_STRING_H__
+
+# include "internal.h"
+
+char **virStringSplit(const char *string,
+ const char *delim,
+ size_t max_tokens)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+
+char *virStringJoin(const char **strings,
+ const char *delim)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+
+void virStringFreeList(char **strings);
+
+#endif /* __VIR_STRING_H__ */
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 705cbb4..1adaa4b 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -100,7 +100,9 @@ test_programs = virshtest sockettest \
virhashtest virnetmessagetest virnetsockettest \
utiltest virnettlscontexttest shunloadtest \
virtimetest viruritest virkeyfiletest \
- virauthconfigtest
+ virauthconfigtest \
+ virstringtest \
+ $(NULL)
# This is a fake SSH we use from virnetsockettest
ssh_SOURCES = ssh.c
@@ -485,6 +487,11 @@ virtimetest_SOURCES = \
virtimetest_CFLAGS = -Dabs_builddir="\"$(abs_builddir)\""
$(AM_CFLAGS)
virtimetest_LDADD = ../src/libvirt-net-rpc.la $(LDADDS)
+virstringtest_SOURCES = \
+ virstringtest.c testutils.h testutils.c
+virstringtest_CFLAGS = -Dabs_builddir="\"$(abs_builddir)\""
$(AM_CFLAGS)
+virstringtest_LDADD = $(LDADDS)
+
viruritest_SOURCES = \
viruritest.c testutils.h testutils.c
viruritest_CFLAGS = -Dabs_builddir="\"$(abs_builddir)\""
$(AM_CFLAGS)
diff --git a/tests/virstringtest.c b/tests/virstringtest.c
new file mode 100644
index 0000000..7e726c6
--- /dev/null
+++ b/tests/virstringtest.c
@@ -0,0 +1,161 @@
+/*
+ * Copyright (C) 2012 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <http://www.gnu.org/licenses/>;.
+ *
+ * Author: Daniel P. Berrange <berrange(a)redhat.com>
+ */
+
+#include <config.h>
+
+#include <stdlib.h>
+
+#include "testutils.h"
+#include "util.h"
+#include "virterror_internal.h"
+#include "memory.h"
+#include "logging.h"
+
+#include "virstring.h"
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+struct testSplitData {
+ const char *string;
+ const char *delim;
+ size_t max_tokens;
+ const char **tokens;
+};
+
+
+struct testJoinData {
+ const char *string;
+ const char *delim;
+ const char **tokens;
+};
+
+static int testSplit(const void *args)
+{
+ const struct testSplitData *data = args;
+ char **got;
+ char **tmp1;
+ const char **tmp2;
+ int ret = -1;
+
+ if (!(got = virStringSplit(data->string, data->delim, data->max_tokens))) {
+ VIR_DEBUG("Got no tokens at all");
+ return -1;
+ }
+
+ tmp1 = got;
+ tmp2 = data->tokens;
+ while (*tmp1 && *tmp2) {
+ if (STRNEQ(*tmp1, *tmp2)) {
+ fprintf(stderr, "Mismatch '%s' vs '%s'\n", *tmp1,
*tmp2);
+ goto cleanup;
+ }
+ tmp1++;
+ tmp2++;
+ }
+ if (*tmp1) {
+ fprintf(stderr, "Too many pieces returned\n");
+ goto cleanup;
+ }
+ if (*tmp2) {
+ fprintf(stderr, "Too few pieces returned\n");
+ goto cleanup;
+ }
+
+ ret = 0;
+cleanup:
+ virStringFreeList(got);
+
+ return ret;
+}
+
+
+static int testJoin(const void *args)
+{
+ const struct testJoinData *data = args;
+ char *got;
+ int ret = -1;
+
+ if (!(got = virStringJoin(data->tokens, data->delim))) {
+ VIR_DEBUG("Got no result");
+ return -1;
+ }
+ if (STRNEQ(got, data->string)) {
+ fprintf(stderr, "Mismatch '%s' vs '%s'\n", got,
data->string);
+ goto cleanup;
+ }
+
+ ret = 0;
+cleanup:
+ VIR_FREE(got);
+
+ return ret;
+}
+
+
+static int
+mymain(void)
+{
+ int ret = 0;
+
+#define TEST_SPLIT(str, del, max, toks) \
+ do { \
+ struct testSplitData splitData = { \
+ .string = str, \
+ .delim = del, \
+ .max_tokens = max, \
+ .tokens = toks, \
+ }; \
+ struct testJoinData joinData = { \
+ .string = str, \
+ .delim = del, \
+ .tokens = toks, \
+ }; \
+ if (virtTestRun("Split " #str, 1, testSplit, &splitData) < 0)
\
+ ret = -1; \
+ if (virtTestRun("Join " #str, 1, testJoin, &joinData) < 0)
\
+ ret = -1; \
+ } while (0)
+
+ const char *tokens1[] = { NULL };
+ TEST_SPLIT("", " ", 0, tokens1);
+
+ const char *tokens2[] = { "", "", NULL };
+ TEST_SPLIT(" ", " ", 0, tokens2);
+
+ const char *tokens3[] = { "", "", "", NULL };
+ TEST_SPLIT(" ", " ", 0, tokens3);
+
+ const char *tokens4[] = { "The", "quick", "brown",
"fox", NULL };
+ TEST_SPLIT("The quick brown fox", " ", 0, tokens4);
+
+ const char *tokens5[] = { "The quick ", " fox", NULL };
+ TEST_SPLIT("The quick brown fox", "brown", 0, tokens5);
+
+ const char *tokens6[] = { "", "The", "quick",
"brown", "fox", NULL };
+ TEST_SPLIT(" The quick brown fox", " ", 0, tokens6);
+
+ const char *tokens7[] = { "The", "quick", "brown",
"fox", "", NULL };
+ TEST_SPLIT("The quick brown fox ", " ", 0, tokens7);
+
+
+ return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE;
+}
+
+VIRT_TEST_MAIN(mymain)
--
1.8.3.1
10:14 p.m.
New subject: [libvirt] [v0.9.12-maint 2/9] Rename virKillProcess to virProcessKill
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Changing naming to follow the convention of "object" followed
by "action"
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
(cherry picked from commit cf470068a114fc7aab5e5de37d3f4fe3545bdc81)
---
src/qemu/qemu_agent.c | 2 +-
src/qemu/qemu_monitor.c | 2 +-
src/qemu/qemu_process.c | 2 +-
src/uml/uml_driver.c | 2 +-
src/util/util.c | 2 +-
src/util/util.h | 2 +-
6 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index bc4ceff..1e3e327 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -224,7 +224,7 @@ qemuAgentOpenUnix(const char *monitor, pid_t cpid, bool *inProgress)
break;
if ((errno == ENOENT || errno == ECONNREFUSED) &&
- virKillProcess(cpid, 0) == 0) {
+ virProcessKill(cpid, 0) == 0) {
/* ENOENT : Socket may not have shown up yet
* ECONNREFUSED : Leftover socket hasn't been removed yet */
continue;
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 68ecdb9..cfe1a25 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -294,7 +294,7 @@ qemuMonitorOpenUnix(const char *monitor, pid_t cpid)
break;
if ((errno == ENOENT || errno == ECONNREFUSED) &&
- virKillProcess(cpid, 0) == 0) {
+ virProcessKill(cpid, 0) == 0) {
/* ENOENT : Socket may not have shown up yet
* ECONNREFUSED : Leftover socket hasn't been removed yet */
continue;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d226067..7fc87f4 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3817,7 +3817,7 @@ qemuProcessKill(struct qemud_driver *driver,
signum = 0; /* Just check for existence */
}
- if (virKillProcess(vm->pid, signum) < 0) {
+ if (virProcessKill(vm->pid, signum) < 0) {
if (errno != ESRCH) {
char ebuf[1024];
VIR_WARN("Failed to terminate process %d with SIG%s: %s",
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 4e640ff..8db78b8 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -1101,7 +1101,7 @@ static void umlShutdownVMDaemon(struct uml_driver *driver,
if (!virDomainObjIsActive(vm))
return;
- virKillProcess(vm->pid, SIGTERM);
+ virProcessKill(vm->pid, SIGTERM);
VIR_FORCE_CLOSE(priv->monitor);
diff --git a/src/util/util.c b/src/util/util.c
index 48358b2..bb15518 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -2133,7 +2133,7 @@ check_and_return:
}
/* send signal to a single process */
-int virKillProcess(pid_t pid, int sig)
+int virProcessKill(pid_t pid, int sig)
{
if (pid <= 1) {
errno = ESRCH;
diff --git a/src/util/util.h b/src/util/util.h
index 85e8bd6..838bff7 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -225,7 +225,7 @@ static inline int getgid (void) { return 0; }
char *virGetHostname(virConnectPtr conn);
-int virKillProcess(pid_t pid, int sig);
+int virProcessKill(pid_t pid, int sig);
char *virGetUserDirectory(uid_t uid);
char *virGetUserName(uid_t uid);
--
1.8.3.1
10:14 p.m.
New subject: [libvirt] [v0.9.12-maint 3/9] Rename virPid{Abort, Wait} to virProcess{Abort, Wait}
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Change "Pid" to "Process" to align with the virProcessKill
API naming prefix
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
(cherry picked from commit 0fb58ef5cd477cf9a0efdd966a22440ef087a2af)
Signed-off-by: Eric Blake <eblake(a)redhat.com>
Conflicts:
src/util/util.c
src/lxc/lxc_container.c
src/lxc/lxc_controller.c
---
daemon/libvirtd.c | 2 +-
src/libvirt_private.syms | 4 ++--
src/lxc/lxc_container.c | 2 +-
src/lxc/lxc_controller.c | 2 +-
src/rpc/virnetsocket.c | 2 +-
src/util/command.c | 24 ++++++++++++------------
src/util/command.h | 6 +++---
src/util/util.c | 6 +++---
tests/testutils.c | 4 ++--
9 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 5d57b50..e9db433 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -192,7 +192,7 @@ static int daemonForkIntoBackground(const char *argv0)
VIR_FORCE_CLOSE(statuspipe[1]);
/* We wait to make sure the first child forked successfully */
- if (virPidWait(pid, NULL) < 0)
+ if (virProcessWait(pid, NULL) < 0)
goto error;
/* If we get here, then the grandchild was spawned, so we
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8328fcf..5d8ff7b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -144,8 +144,8 @@ virCommandTranslateStatus;
virCommandWait;
virCommandWriteArgLog;
virFork;
-virPidAbort;
-virPidWait;
+virProcessAbort;
+virProcessWait;
virRun;
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 0636eab..442aa24 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -1519,7 +1519,7 @@ int lxcContainerAvailable(int features)
VIR_DEBUG("clone call returned %s, container support is not enabled",
virStrerror(errno, ebuf, sizeof(ebuf)));
return -1;
- } else if (virPidWait(cpid, NULL) < 0) {
+ } else if (virProcessWait(cpid, NULL) < 0) {
return -1;
}
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 26b3115..b5cec3d 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -1626,7 +1626,7 @@ cleanup:
VIR_FORCE_CLOSE(loopDevs[i]);
VIR_FREE(loopDevs);
- virPidAbort(container);
+ virProcessAbort(container);
return rc;
}
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index fa16d31..040090e 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -752,7 +752,7 @@ void virNetSocketFree(virNetSocketPtr sock)
VIR_FORCE_CLOSE(sock->fd);
VIR_FORCE_CLOSE(sock->errfd);
- virPidAbort(sock->pid);
+ virProcessAbort(sock->pid);
VIR_FREE(sock->localAddrStr);
VIR_FREE(sock->remoteAddrStr);
diff --git a/src/util/command.c b/src/util/command.c
index eaa9f16..4340659 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -1652,7 +1652,7 @@ virCommandToString(virCommandPtr cmd)
* @status: child exit status to translate
*
* Translate an exit status into a malloc'd string. Generic helper
- * for virCommandRun(), virCommandWait(), virRun(), and virPidWait()
+ * for virCommandRun(), virCommandWait(), virRun(), and virProcessWait()
* status argument, as well as raw waitpid().
*/
char *
@@ -2114,7 +2114,7 @@ virCommandHook(void *data)
* for pid. While cmd is still in scope, you may reap the child via
* virCommandWait or virCommandAbort. But after virCommandFree, if
* you have not yet reaped the child, then it continues to run until
- * you call virPidWait or virPidAbort.
+ * you call virProcessWait or virProcessAbort.
*/
int
virCommandRunAsync(virCommandPtr cmd, pid_t *pid)
@@ -2207,7 +2207,7 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid)
/**
- * virPidWait:
+ * virProcessWait:
* @pid: child to wait on
* @exitstatus: optional status collection
*
@@ -2218,7 +2218,7 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid)
* child must exit with status 0 for this to succeed.
*/
int
-virPidWait(pid_t pid, int *exitstatus)
+virProcessWait(pid_t pid, int *exitstatus)
{
int ret;
int status;
@@ -2288,13 +2288,13 @@ virCommandWait(virCommandPtr cmd, int *exitstatus)
return -1;
}
- /* If virPidWait reaps pid but then returns failure because
+ /* If virProcessWait reaps pid but then returns failure because
* exitstatus was NULL, then a second virCommandWait would risk
* calling waitpid on an unrelated process. Besides, that error
* message is not as detailed as what we can provide. So, we
- * guarantee that virPidWait only fails due to failure to wait,
+ * guarantee that virProcessWait only fails due to failure to wait,
* and repeat the exitstatus check code ourselves. */
- ret = virPidWait(cmd->pid, exitstatus ? exitstatus : &status);
+ ret = virProcessWait(cmd->pid, exitstatus ? exitstatus : &status);
if (ret == 0) {
cmd->pid = -1;
cmd->reap = false;
@@ -2316,7 +2316,7 @@ virCommandWait(virCommandPtr cmd, int *exitstatus)
#ifndef WIN32
/**
- * virPidAbort:
+ * virProcessAbort:
* @pid: child process to kill
*
* Abort a child process if PID is positive and that child is still
@@ -2326,7 +2326,7 @@ virCommandWait(virCommandPtr cmd, int *exitstatus)
* this does nothing.
*/
void
-virPidAbort(pid_t pid)
+virProcessAbort(pid_t pid)
{
int saved_errno;
int ret;
@@ -2390,13 +2390,13 @@ virCommandAbort(virCommandPtr cmd)
{
if (!cmd || cmd->pid == -1)
return;
- virPidAbort(cmd->pid);
+ virProcessAbort(cmd->pid);
cmd->pid = -1;
cmd->reap = false;
}
#else /* WIN32 */
void
-virPidAbort(pid_t pid)
+virProcessAbort(pid_t pid)
{
/* Not yet ported to mingw. Any volunteers? */
VIR_DEBUG("failed to reap child %lld, abandoning it", (long long)pid);
@@ -2553,7 +2553,7 @@ int virCommandHandshakeNotify(virCommandPtr cmd)
*
* Release all resources. The only exception is that if you called
* virCommandRunAsync with a non-null pid, then the asynchronous child
- * is not reaped, and you must call virPidWait() or virPidAbort() yourself.
+ * is not reaped, and you must call virProcessWait() or virProcessAbort() yourself.
*/
void
virCommandFree(virCommandPtr cmd)
diff --git a/src/util/command.h b/src/util/command.h
index 07aa0b3..d8a8c83 100644
--- a/src/util/command.h
+++ b/src/util/command.h
@@ -148,8 +148,8 @@ int virCommandRun(virCommandPtr cmd,
int virCommandRunAsync(virCommandPtr cmd,
pid_t *pid) ATTRIBUTE_RETURN_CHECK;
-int virPidWait(pid_t pid,
- int *exitstatus) ATTRIBUTE_RETURN_CHECK;
+int virProcessWait(pid_t pid,
+ int *exitstatus) ATTRIBUTE_RETURN_CHECK;
int virCommandWait(virCommandPtr cmd,
int *exitstatus) ATTRIBUTE_RETURN_CHECK;
@@ -162,7 +162,7 @@ int virCommandHandshakeWait(virCommandPtr cmd)
int virCommandHandshakeNotify(virCommandPtr cmd)
ATTRIBUTE_RETURN_CHECK;
-void virPidAbort(pid_t pid);
+void virProcessAbort(pid_t pid);
void virCommandAbort(virCommandPtr cmd);
diff --git a/src/util/util.c b/src/util/util.c
index bb15518..e1f8d1e 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -718,10 +718,10 @@ virFileAccessibleAs(const char *path, int mode,
}
if (pid) { /* parent */
- if (virPidWait(pid, &status) < 0) {
- /* virPidWait() already
+ if (virProcessWait(pid, &status) < 0) {
+ /* virProcessWait() already
* reported error */
- return -1;
+ return -1;
}
if (!WIFEXITED(status)) {
diff --git a/tests/testutils.c b/tests/testutils.c
index 6eb40ed..63bec52 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -314,7 +314,7 @@ virtTestCaptureProgramOutput(const char *const argv[], char **buf, int
maxlen)
VIR_FORCE_CLOSE(pipefd[1]);
len = virFileReadLimFD(pipefd[0], maxlen, buf);
VIR_FORCE_CLOSE(pipefd[0]);
- if (virPidWait(pid, NULL) < 0)
+ if (virProcessWait(pid, NULL) < 0)
return -1;
return len;
@@ -684,7 +684,7 @@ int virtTestMain(int argc,
} else {
int i, status;
for (i = 0 ; i < mp ; i++) {
- if (virPidWait(workers[i], NULL) < 0)
+ if (virProcessWait(workers[i], NULL) < 0)
ret = EXIT_FAILURE;
}
VIR_FREE(workers);
--
1.8.3.1
10:14 p.m.
New subject: [libvirt] [v0.9.12-maint 4/9] Rename virCommandTranslateStatus to virProcessTranslateStatus
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The virCommand prefix was inappropriate because the API
does not use any virCommandPtr object instance. This
API closely related to waitpid/exit, so use virProcess
as the prefix
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
(cherry picked from commit 49ecf8b41fd1c606703d01792701ce46352b7669)
Signed-off-by: Eric Blake <eblake(a)redhat.com>
Conflicts:
src/util/command.c
---
daemon/remote.c | 2 +-
src/libvirt_private.syms | 2 +-
src/util/command.c | 24 ++++++++++++++----------
src/util/command.h | 2 +-
4 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index 4ece019..24553f0 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -2600,7 +2600,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED,
authdismissed = (pkout && strstr(pkout, "dismissed=true"));
if (status != 0) {
- char *tmp = virCommandTranslateStatus(status);
+ char *tmp = virProcessTranslateStatus(status);
VIR_ERROR(_("Policy kit denied action %s from pid %lld, uid %d: %s"),
action, (long long) callerPid, callerUid, NULLSTR(tmp));
VIR_FREE(tmp);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 5d8ff7b..13a732b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -140,7 +140,7 @@ virCommandSetPreExecHook;
virCommandSetWorkingDirectory;
virCommandToString;
virCommandTransferFD;
-virCommandTranslateStatus;
+virProcessTranslateStatus;
virCommandWait;
virCommandWriteArgLog;
virFork;
diff --git a/src/util/command.c b/src/util/command.c
index 4340659..02432fa 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -1648,7 +1648,7 @@ virCommandToString(virCommandPtr cmd)
/**
- * virCommandTranslateStatus:
+ * virProcessTranslateStatus:
* @status: child exit status to translate
*
* Translate an exit status into a malloc'd string. Generic helper
@@ -1656,7 +1656,7 @@ virCommandToString(virCommandPtr cmd)
* status argument, as well as raw waitpid().
*/
char *
-virCommandTranslateStatus(int status)
+virProcessTranslateStatus(int status)
{
char *buf;
if (WIFEXITED(status)) {
@@ -1986,7 +1986,7 @@ virCommandRun(virCommandPtr cmd, int *exitstatus)
if (virCommandWait(cmd, exitstatus) < 0)
ret = -1;
- str = (exitstatus ? virCommandTranslateStatus(*exitstatus)
+ str = (exitstatus ? virProcessTranslateStatus(*exitstatus)
: (char *) "status 0");
VIR_DEBUG("Result %s, stdout: '%s' stderr: '%s'",
NULLSTR(str),
@@ -2241,7 +2241,7 @@ virProcessWait(pid_t pid, int *exitstatus)
if (exitstatus == NULL) {
if (status != 0) {
- char *st = virCommandTranslateStatus(status);
+ char *st = virProcessTranslateStatus(status);
virCommandError(VIR_ERR_INTERNAL_ERROR,
_("Child process (%lld) status unexpected: %s"),
(long long) pid, NULLSTR(st));
@@ -2300,10 +2300,14 @@ virCommandWait(virCommandPtr cmd, int *exitstatus)
cmd->reap = false;
if (status) {
char *str = virCommandToString(cmd);
- char *st = virCommandTranslateStatus(status);
+ char *st = virProcessTranslateStatus(status);
+ bool haveErrMsg = cmd->errbuf && *cmd->errbuf &&
(*cmd->errbuf)[0];
+
virCommandError(VIR_ERR_INTERNAL_ERROR,
- _("Child process (%s) status unexpected: %s"),
- str ? str : cmd->args[0], NULLSTR(st));
+ _("Child process (%s) unexpected %s%s%s"),
+ str ? str : cmd->args[0], NULLSTR(st),
+ haveErrMsg ? ": " : "",
+ haveErrMsg ? *cmd->errbuf : "");
VIR_FREE(str);
VIR_FREE(st);
return -1;
@@ -2344,7 +2348,7 @@ virProcessAbort(pid_t pid)
while ((ret = waitpid(pid, &status, WNOHANG)) == -1 &&
errno == EINTR);
if (ret == pid) {
- tmp = virCommandTranslateStatus(status);
+ tmp = virProcessTranslateStatus(status);
VIR_DEBUG("process has ended: %s", tmp);
goto cleanup;
} else if (ret == 0) {
@@ -2354,7 +2358,7 @@ virProcessAbort(pid_t pid)
while ((ret = waitpid(pid, &status, WNOHANG)) == -1 &&
errno == EINTR);
if (ret == pid) {
- tmp = virCommandTranslateStatus(status);
+ tmp = virProcessTranslateStatus(status);
VIR_DEBUG("process has ended: %s", tmp);
goto cleanup;
} else if (ret == 0) {
@@ -2363,7 +2367,7 @@ virProcessAbort(pid_t pid)
while ((ret = waitpid(pid, &status, 0)) == -1 &&
errno == EINTR);
if (ret == pid) {
- tmp = virCommandTranslateStatus(status);
+ tmp = virProcessTranslateStatus(status);
VIR_DEBUG("process has ended: %s", tmp);
goto cleanup;
}
diff --git a/src/util/command.h b/src/util/command.h
index d8a8c83..5cd85e5 100644
--- a/src/util/command.h
+++ b/src/util/command.h
@@ -138,7 +138,7 @@ void virCommandWriteArgLog(virCommandPtr cmd,
char *virCommandToString(virCommandPtr cmd) ATTRIBUTE_RETURN_CHECK;
-char *virCommandTranslateStatus(int exitstatus) ATTRIBUTE_RETURN_CHECK;
+char *virProcessTranslateStatus(int exitstatus) ATTRIBUTE_RETURN_CHECK;
int virCommandExec(virCommandPtr cmd) ATTRIBUTE_RETURN_CHECK;
--
1.8.3.1
10:14 p.m.
New subject: [libvirt] [v0.9.12-maint 5/9] Move virProcessKill into virprocess.{h, c}
From: "Daniel P. Berrange" <berrange(a)redhat.com>
There are a number of process related functions spread
across multiple files. Start to consolidate them by
creating a virprocess.{c,h} file
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
(cherry picked from commit e5e2b65cf86ea49eba76b3c274e3b9d2177485bc)
Signed-off-by: Eric Blake <eblake(a)redhat.com>
Conflicts:
src/qemu/qemu_monitor.c
src/util/util.h
---
src/Makefile.am | 1 +
src/libvirt_private.syms | 3 ++
src/qemu/qemu_agent.c | 1 +
src/qemu/qemu_monitor.c | 1 +
src/qemu/qemu_process.c | 1 +
src/uml/uml_driver.c | 1 +
src/util/util.c | 57 --------------------------------
src/util/util.h | 1 -
src/util/virprocess.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++
src/util/virprocess.h | 31 ++++++++++++++++++
10 files changed, 123 insertions(+), 58 deletions(-)
create mode 100644 src/util/virprocess.c
create mode 100644 src/util/virprocess.h
diff --git a/src/Makefile.am b/src/Makefile.am
index b5da2fb..6656fa0 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -85,6 +85,7 @@ UTIL_SOURCES = \
util/virfile.c util/virfile.h \
util/virnodesuspend.c util/virnodesuspend.h \
util/virpidfile.c util/virpidfile.h \
+ util/virprocess.c util/virprocess.h \
util/virtypedparam.c util/virtypedparam.h \
util/xml.c util/xml.h \
util/virterror.c util/virterror_internal.h \
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 13a732b..95f2543 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1453,6 +1453,9 @@ virPidFileDelete;
virPidFileDeletePath;
+# virprocess.h
+virProcessKill;
+
# virrandom.h
virRandomBits;
virRandomGenerateWWN;
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 1e3e327..5618ddf 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -39,6 +39,7 @@
#include "virterror_internal.h"
#include "json.h"
#include "virfile.h"
+#include "virprocess.h"
#include "virtime.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index cfe1a25..265f03c 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -36,6 +36,7 @@
#include "memory.h"
#include "logging.h"
#include "virfile.h"
+#include "virprocess.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 7fc87f4..a14fc57 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -61,6 +61,7 @@
#include "locking/domain_lock.h"
#include "network/bridge_driver.h"
#include "uuid.h"
+#include "virprocess.h"
#include "virtime.h"
#include "virnetdevtap.h"
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 8db78b8..83a31a9 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -63,6 +63,7 @@
#include "configmake.h"
#include "virnetdevtap.h"
#include "virnodesuspend.h"
+#include "virprocess.h"
#include "viruri.h"
#define VIR_FROM_THIS VIR_FROM_UML
diff --git a/src/util/util.c b/src/util/util.c
index e1f8d1e..b807c5a 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -2132,63 +2132,6 @@ check_and_return:
return result;
}
-/* send signal to a single process */
-int virProcessKill(pid_t pid, int sig)
-{
- if (pid <= 1) {
- errno = ESRCH;
- return -1;
- }
-
-#ifdef WIN32
- /* Mingw / Windows don't have many signals (AFAIK) */
- switch (sig) {
- case SIGINT:
- /* This does a Ctrl+C equiv */
- if (!GenerateConsoleCtrlEvent(CTRL_C_EVENT, pid)) {
- errno = ESRCH;
- return -1;
- }
- break;
-
- case SIGTERM:
- /* Since TerminateProcess is closer to SIG_KILL, we do
- * a Ctrl+Break equiv which is more pleasant like the
- * good old unix SIGTERM/HUP
- */
- if (!GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT, pid)) {
- errno = ESRCH;
- return -1;
- }
- break;
-
- default:
- {
- HANDLE proc;
- proc = OpenProcess(PROCESS_TERMINATE, FALSE, pid);
- if (!proc) {
- errno = ESRCH; /* Not entirely accurate, but close enough */
- return -1;
- }
-
- /*
- * TerminateProcess is more or less equiv to SIG_KILL, in that
- * a process can't trap / block it
- */
- if (sig != 0 && !TerminateProcess(proc, sig)) {
- errno = ESRCH;
- return -1;
- }
- CloseHandle(proc);
- }
- }
- return 0;
-#else
- return kill(pid, sig);
-#endif
-}
-
-
#ifdef HAVE_GETPWUID_R
enum {
VIR_USER_ENT_DIRECTORY,
diff --git a/src/util/util.h b/src/util/util.h
index 838bff7..c63f5fb 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -225,7 +225,6 @@ static inline int getgid (void) { return 0; }
char *virGetHostname(virConnectPtr conn);
-int virProcessKill(pid_t pid, int sig);
char *virGetUserDirectory(uid_t uid);
char *virGetUserName(uid_t uid);
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
new file mode 100644
index 0000000..e7db68f
--- /dev/null
+++ b/src/util/virprocess.c
@@ -0,0 +1,84 @@
+/*
+ * virprocess.c: interaction with processes
+ *
+ * Copyright (C) 2010-2012 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <http://www.gnu.org/licenses/>;.
+ *
+ */
+
+
+#include <config.h>
+
+#include <signal.h>
+#include <errno.h>
+
+#include "virprocess.h"
+
+/* send signal to a single process */
+int virProcessKill(pid_t pid, int sig)
+{
+ if (pid <= 1) {
+ errno = ESRCH;
+ return -1;
+ }
+
+#ifdef WIN32
+ /* Mingw / Windows don't have many signals (AFAIK) */
+ switch (sig) {
+ case SIGINT:
+ /* This does a Ctrl+C equiv */
+ if (!GenerateConsoleCtrlEvent(CTRL_C_EVENT, pid)) {
+ errno = ESRCH;
+ return -1;
+ }
+ break;
+
+ case SIGTERM:
+ /* Since TerminateProcess is closer to SIG_KILL, we do
+ * a Ctrl+Break equiv which is more pleasant like the
+ * good old unix SIGTERM/HUP
+ */
+ if (!GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT, pid)) {
+ errno = ESRCH;
+ return -1;
+ }
+ break;
+
+ default:
+ {
+ HANDLE proc;
+ proc = OpenProcess(PROCESS_TERMINATE, FALSE, pid);
+ if (!proc) {
+ errno = ESRCH; /* Not entirely accurate, but close enough */
+ return -1;
+ }
+
+ /*
+ * TerminateProcess is more or less equiv to SIG_KILL, in that
+ * a process can't trap / block it
+ */
+ if (sig != 0 && !TerminateProcess(proc, sig)) {
+ errno = ESRCH;
+ return -1;
+ }
+ CloseHandle(proc);
+ }
+ }
+ return 0;
+#else
+ return kill(pid, sig);
+#endif
+}
diff --git a/src/util/virprocess.h b/src/util/virprocess.h
new file mode 100644
index 0000000..b1000c6
--- /dev/null
+++ b/src/util/virprocess.h
@@ -0,0 +1,31 @@
+/*
+ * virprocess.h: interaction with processes
+ *
+ * Copyright (C) 2010-2012 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <http://www.gnu.org/licenses/>;.
+ *
+ */
+
+#ifndef __VIR_PROCESS_H__
+# define __VIR_PROCESS_H__
+
+# include <sys/types.h>
+
+# include "internal.h"
+
+int virProcessKill(pid_t pid, int sig);
+
+#endif /* __VIR_PROCESS_H__ */
--
1.8.3.1
10:14 p.m.
New subject: [libvirt] [v0.9.12-maint 6/9] Move virProcess{Kill, Abort, TranslateStatus} into virprocess.{c, h}
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Continue consolidation of process functions by moving some
helpers out of command.{c,h} into virprocess.{c,h}
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
(cherry picked from commit 9467ab6074d02bd90248b5710b1c83856fefe9b4)
Signed-off-by: Eric Blake <eblake(a)redhat.com>
Conflicts:
src/lxc/lxc_controller.c
src/util/command.c
src/util/virprocess.c
tests/testutils.c
---
daemon/libvirtd.c | 1 +
daemon/remote.c | 1 +
po/POTFILES.in | 1 +
src/libvirt_private.syms | 7 ++-
src/lxc/lxc_container.c | 1 +
src/lxc/lxc_controller.c | 1 +
src/rpc/virnetsocket.c | 1 +
src/util/command.c | 143 +-----------------------------------------
src/util/command.h | 8 ---
src/util/util.c | 1 +
src/util/virprocess.c | 157 +++++++++++++++++++++++++++++++++++++++++++++++
src/util/virprocess.h | 11 ++++
tests/testutils.c | 1 +
13 files changed, 181 insertions(+), 153 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index e9db433..49faece 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -36,6 +36,7 @@
#include "virterror_internal.h"
#include "virfile.h"
#include "virpidfile.h"
+#include "virprocess.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
diff --git a/daemon/remote.c b/daemon/remote.c
index 24553f0..435f663 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -46,6 +46,7 @@
#include "virfile.h"
#include "virtypedparam.h"
#include "virdbus.h"
+#include "virprocess.h"
#include "remote_protocol.h"
#include "qemu_protocol.h"
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 4ea544b..7f1997c 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -141,6 +141,7 @@ src/util/virnetdevvportprofile.c
src/util/virnetlink.c
src/util/virnodesuspend.c
src/util/virpidfile.c
+src/util/virprocess.c
src/util/virrandom.c
src/util/virsocketaddr.c
src/util/virterror.c
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 95f2543..f35fd63 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -140,12 +140,9 @@ virCommandSetPreExecHook;
virCommandSetWorkingDirectory;
virCommandToString;
virCommandTransferFD;
-virProcessTranslateStatus;
virCommandWait;
virCommandWriteArgLog;
virFork;
-virProcessAbort;
-virProcessWait;
virRun;
@@ -1454,7 +1451,11 @@ virPidFileDeletePath;
# virprocess.h
+virProcessAbort;
virProcessKill;
+virProcessTranslateStatus;
+virProcessWait;
+
# virrandom.h
virRandomBits;
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 442aa24..b0bd7a2 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -64,6 +64,7 @@
#include "virfile.h"
#include "command.h"
#include "virnetdev.h"
+#include "virprocess.h"
#define VIR_FROM_THIS VIR_FROM_LXC
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index b5cec3d..e6a2917 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -71,6 +71,7 @@
#include "command.h"
#include "processinfo.h"
#include "nodeinfo.h"
+#include "virprocess.h"
#define VIR_FROM_THIS VIR_FROM_LXC
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 040090e..e82f375 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -43,6 +43,7 @@
#include "virfile.h"
#include "event.h"
#include "threads.h"
+#include "virprocess.h"
#include "passfd.h"
diff --git a/src/util/command.c b/src/util/command.c
index 02432fa..be10131 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -40,6 +40,7 @@
#include "logging.h"
#include "virfile.h"
#include "virpidfile.h"
+#include "virprocess.h"
#include "buf.h"
#include "ignore-value.h"
@@ -1647,31 +1648,6 @@ virCommandToString(virCommandPtr cmd)
}
-/**
- * virProcessTranslateStatus:
- * @status: child exit status to translate
- *
- * Translate an exit status into a malloc'd string. Generic helper
- * for virCommandRun(), virCommandWait(), virRun(), and virProcessWait()
- * status argument, as well as raw waitpid().
- */
-char *
-virProcessTranslateStatus(int status)
-{
- char *buf;
- if (WIFEXITED(status)) {
- ignore_value(virAsprintf(&buf, _("exit status %d"),
- WEXITSTATUS(status)));
- } else if (WIFSIGNALED(status)) {
- ignore_value(virAsprintf(&buf, _("fatal signal %d"),
- WTERMSIG(status)));
- } else {
- ignore_value(virAsprintf(&buf, _("invalid value %d"), status));
- }
- return buf;
-}
-
-
/*
* Manage input and output to the child process.
*/
@@ -2207,55 +2183,6 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid)
/**
- * virProcessWait:
- * @pid: child to wait on
- * @exitstatus: optional status collection
- *
- * Wait for a child process to complete.
- * Return -1 on any error waiting for
- * completion. Returns 0 if the command
- * finished with the exit status set. If @exitstatus is NULL, then the
- * child must exit with status 0 for this to succeed.
- */
-int
-virProcessWait(pid_t pid, int *exitstatus)
-{
- int ret;
- int status;
-
- if (pid <= 0) {
- virReportSystemError(EINVAL, _("unable to wait for process %lld"),
- (long long) pid);
- return -1;
- }
-
- /* Wait for intermediate process to exit */
- while ((ret = waitpid(pid, &status, 0)) == -1 &&
- errno == EINTR);
-
- if (ret == -1) {
- virReportSystemError(errno, _("unable to wait for process %lld"),
- (long long) pid);
- return -1;
- }
-
- if (exitstatus == NULL) {
- if (status != 0) {
- char *st = virProcessTranslateStatus(status);
- virCommandError(VIR_ERR_INTERNAL_ERROR,
- _("Child process (%lld) status unexpected: %s"),
- (long long) pid, NULLSTR(st));
- VIR_FREE(st);
- return -1;
- }
- } else {
- *exitstatus = status;
- }
-
- return 0;
-}
-
-/**
* virCommandWait:
* @cmd: command to wait on
* @exitstatus: optional status collection
@@ -2320,67 +2247,6 @@ virCommandWait(virCommandPtr cmd, int *exitstatus)
#ifndef WIN32
/**
- * virProcessAbort:
- * @pid: child process to kill
- *
- * Abort a child process if PID is positive and that child is still
- * running, without issuing any errors or affecting errno. Designed
- * for error paths where some but not all paths to the cleanup code
- * might have started the child process. If @pid is 0 or negative,
- * this does nothing.
- */
-void
-virProcessAbort(pid_t pid)
-{
- int saved_errno;
- int ret;
- int status;
- char *tmp = NULL;
-
- if (pid <= 0)
- return;
-
- /* See if intermediate process has exited; if not, try a nice
- * SIGTERM followed by a more severe SIGKILL.
- */
- saved_errno = errno;
- VIR_DEBUG("aborting child process %d", pid);
- while ((ret = waitpid(pid, &status, WNOHANG)) == -1 &&
- errno == EINTR);
- if (ret == pid) {
- tmp = virProcessTranslateStatus(status);
- VIR_DEBUG("process has ended: %s", tmp);
- goto cleanup;
- } else if (ret == 0) {
- VIR_DEBUG("trying SIGTERM to child process %d", pid);
- kill(pid, SIGTERM);
- usleep(10 * 1000);
- while ((ret = waitpid(pid, &status, WNOHANG)) == -1 &&
- errno == EINTR);
- if (ret == pid) {
- tmp = virProcessTranslateStatus(status);
- VIR_DEBUG("process has ended: %s", tmp);
- goto cleanup;
- } else if (ret == 0) {
- VIR_DEBUG("trying SIGKILL to child process %d", pid);
- kill(pid, SIGKILL);
- while ((ret = waitpid(pid, &status, 0)) == -1 &&
- errno == EINTR);
- if (ret == pid) {
- tmp = virProcessTranslateStatus(status);
- VIR_DEBUG("process has ended: %s", tmp);
- goto cleanup;
- }
- }
- }
- VIR_DEBUG("failed to reap child %lld, abandoning it", (long long) pid);
-
-cleanup:
- VIR_FREE(tmp);
- errno = saved_errno;
-}
-
-/**
* virCommandAbort:
* @cmd: command to abort
*
@@ -2400,13 +2266,6 @@ virCommandAbort(virCommandPtr cmd)
}
#else /* WIN32 */
void
-virProcessAbort(pid_t pid)
-{
- /* Not yet ported to mingw. Any volunteers? */
- VIR_DEBUG("failed to reap child %lld, abandoning it", (long long)pid);
-}
-
-void
virCommandAbort(virCommandPtr cmd ATTRIBUTE_UNUSED)
{
/* Mingw lacks WNOHANG and kill(). But since we haven't ported
diff --git a/src/util/command.h b/src/util/command.h
index 5cd85e5..cf4b041 100644
--- a/src/util/command.h
+++ b/src/util/command.h
@@ -137,9 +137,6 @@ void virCommandWriteArgLog(virCommandPtr cmd,
char *virCommandToString(virCommandPtr cmd) ATTRIBUTE_RETURN_CHECK;
-
-char *virProcessTranslateStatus(int exitstatus) ATTRIBUTE_RETURN_CHECK;
-
int virCommandExec(virCommandPtr cmd) ATTRIBUTE_RETURN_CHECK;
int virCommandRun(virCommandPtr cmd,
@@ -148,9 +145,6 @@ int virCommandRun(virCommandPtr cmd,
int virCommandRunAsync(virCommandPtr cmd,
pid_t *pid) ATTRIBUTE_RETURN_CHECK;
-int virProcessWait(pid_t pid,
- int *exitstatus) ATTRIBUTE_RETURN_CHECK;
-
int virCommandWait(virCommandPtr cmd,
int *exitstatus) ATTRIBUTE_RETURN_CHECK;
@@ -162,8 +156,6 @@ int virCommandHandshakeWait(virCommandPtr cmd)
int virCommandHandshakeNotify(virCommandPtr cmd)
ATTRIBUTE_RETURN_CHECK;
-void virProcessAbort(pid_t pid);
-
void virCommandAbort(virCommandPtr cmd);
void virCommandFree(virCommandPtr cmd);
diff --git a/src/util/util.c b/src/util/util.c
index b807c5a..1b32bcc 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -78,6 +78,7 @@
#include "command.h"
#include "nonblocking.h"
#include "passfd.h"
+#include "virprocess.h"
#ifndef NSIG
# define NSIG 32
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index e7db68f..ac78f1d 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -24,8 +24,165 @@
#include <signal.h>
#include <errno.h>
+#include <sys/wait.h>
#include "virprocess.h"
+#include "virterror_internal.h"
+#include "memory.h"
+#include "logging.h"
+#include "util.h"
+#include "ignore-value.h"
+#define virReportError(code, ...) \
+ virReportErrorHelper(VIR_FROM_NONE, code, __FILE__, \
+ __FUNCTION__, __LINE__, __VA_ARGS__)
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+/**
+ * virProcessTranslateStatus:
+ * @status: child exit status to translate
+ *
+ * Translate an exit status into a malloc'd string. Generic helper
+ * for virCommandRun(), virCommandWait(), virRun(), and virProcessWait()
+ * status argument, as well as raw waitpid().
+ */
+char *
+virProcessTranslateStatus(int status)
+{
+ char *buf;
+ if (WIFEXITED(status)) {
+ ignore_value(virAsprintf(&buf, _("exit status %d"),
+ WEXITSTATUS(status)));
+ } else if (WIFSIGNALED(status)) {
+ ignore_value(virAsprintf(&buf, _("fatal signal %d"),
+ WTERMSIG(status)));
+ } else {
+ ignore_value(virAsprintf(&buf, _("invalid value %d"), status));
+ }
+ return buf;
+}
+
+
+#ifndef WIN32
+/**
+ * virProcessAbort:
+ * @pid: child process to kill
+ *
+ * Abort a child process if PID is positive and that child is still
+ * running, without issuing any errors or affecting errno. Designed
+ * for error paths where some but not all paths to the cleanup code
+ * might have started the child process. If @pid is 0 or negative,
+ * this does nothing.
+ */
+void
+virProcessAbort(pid_t pid)
+{
+ int saved_errno;
+ int ret;
+ int status;
+ char *tmp = NULL;
+
+ if (pid <= 0)
+ return;
+
+ /* See if intermediate process has exited; if not, try a nice
+ * SIGTERM followed by a more severe SIGKILL.
+ */
+ saved_errno = errno;
+ VIR_DEBUG("aborting child process %d", pid);
+ while ((ret = waitpid(pid, &status, WNOHANG)) == -1 &&
+ errno == EINTR);
+ if (ret == pid) {
+ tmp = virProcessTranslateStatus(status);
+ VIR_DEBUG("process has ended: %s", tmp);
+ goto cleanup;
+ } else if (ret == 0) {
+ VIR_DEBUG("trying SIGTERM to child process %d", pid);
+ kill(pid, SIGTERM);
+ usleep(10 * 1000);
+ while ((ret = waitpid(pid, &status, WNOHANG)) == -1 &&
+ errno == EINTR);
+ if (ret == pid) {
+ tmp = virProcessTranslateStatus(status);
+ VIR_DEBUG("process has ended: %s", tmp);
+ goto cleanup;
+ } else if (ret == 0) {
+ VIR_DEBUG("trying SIGKILL to child process %d", pid);
+ kill(pid, SIGKILL);
+ while ((ret = waitpid(pid, &status, 0)) == -1 &&
+ errno == EINTR);
+ if (ret == pid) {
+ tmp = virProcessTranslateStatus(status);
+ VIR_DEBUG("process has ended: %s", tmp);
+ goto cleanup;
+ }
+ }
+ }
+ VIR_DEBUG("failed to reap child %lld, abandoning it", (long long) pid);
+
+cleanup:
+ VIR_FREE(tmp);
+ errno = saved_errno;
+}
+#else
+void
+virProcessAbort(pid_t pid)
+{
+ /* Not yet ported to mingw. Any volunteers? */
+ VIR_DEBUG("failed to reap child %lld, abandoning it", (long long)pid);
+}
+#endif
+
+
+/**
+ * virProcessWait:
+ * @pid: child to wait on
+ * @exitstatus: optional status collection
+ *
+ * Wait for a child process to complete.
+ * Return -1 on any error waiting for
+ * completion. Returns 0 if the command
+ * finished with the exit status set. If @exitstatus is NULL, then the
+ * child must exit with status 0 for this to succeed.
+ */
+int
+virProcessWait(pid_t pid, int *exitstatus)
+{
+ int ret;
+ int status;
+
+ if (pid <= 0) {
+ virReportSystemError(EINVAL, _("unable to wait for process %lld"),
+ (long long) pid);
+ return -1;
+ }
+
+ /* Wait for intermediate process to exit */
+ while ((ret = waitpid(pid, &status, 0)) == -1 &&
+ errno == EINTR);
+
+ if (ret == -1) {
+ virReportSystemError(errno, _("unable to wait for process %lld"),
+ (long long) pid);
+ return -1;
+ }
+
+ if (exitstatus == NULL) {
+ if (status != 0) {
+ char *st = virProcessTranslateStatus(status);
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Child process (%lld) unexpected %s"),
+ (long long) pid, NULLSTR(st));
+ VIR_FREE(st);
+ return -1;
+ }
+ } else {
+ *exitstatus = status;
+ }
+
+ return 0;
+}
+
/* send signal to a single process */
int virProcessKill(pid_t pid, int sig)
diff --git a/src/util/virprocess.h b/src/util/virprocess.h
index b1000c6..048a73c 100644
--- a/src/util/virprocess.h
+++ b/src/util/virprocess.h
@@ -26,6 +26,17 @@
# include "internal.h"
+char *
+virProcessTranslateStatus(int status);
+
+void
+virProcessAbort(pid_t pid);
+
+int
+virProcessWait(pid_t pid, int *exitstatus)
+ ATTRIBUTE_RETURN_CHECK;
+
int virProcessKill(pid_t pid, int sig);
+
#endif /* __VIR_PROCESS_H__ */
diff --git a/tests/testutils.c b/tests/testutils.c
index 63bec52..2c38115 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -35,6 +35,7 @@
#include "logging.h"
#include "command.h"
#include "virrandom.h"
+#include "virprocess.h"
#if TEST_OOM_TRACE
# include <execinfo.h>
--
1.8.3.1
10:14 p.m.
New subject: [libvirt] [v0.9.12-maint 7/9] Include process start time when doing polkit checks
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Since PIDs can be reused, polkit prefers to be given
a (PID,start time) pair. If given a PID on its own,
it will attempt to lookup the start time in /proc/pid/stat,
though this is subject to races.
It is safer if the client app resolves the PID start
time itself, because as long as the app has the client
socket open, the client PID won't be reused.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
(cherry picked from commit 979e9c56a7aadf2dcfbddd1abfbad594b78b4468)
Signed-off-by: Eric Blake <eblake(a)redhat.com>
Conflicts:
src/libvirt_private.syms - not backported
src/locking/lock_daemon.c - not backported
src/rpc/virnetserverclient.c
src/rpc/virnetsocket.c
src/rpc/virnetsocket.h
src/util/viridentity.h - not backported
src/util/virprocess.c
src/util/virprocess.h
src/util/virstring.c
src/util/virstring.h
Most conflicts were contextual (this patch adds new functions,
but upstream intermediate patches not backported here also added
new features, and the resolution was picking out just the portions
needed by this commit). virnetsocket.c also had slightly
different locking semantics.
---
daemon/remote.c | 12 +++--
src/rpc/virnetserverclient.c | 8 ++-
src/rpc/virnetserverclient.h | 3 +-
src/rpc/virnetsocket.c | 19 +++++--
src/rpc/virnetsocket.h | 3 +-
src/util/virprocess.c | 118 +++++++++++++++++++++++++++++++++++++++++++
src/util/virprocess.h | 3 ++
src/util/virstring.c | 11 ++++
src/util/virstring.h | 2 +
9 files changed, 167 insertions(+), 12 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index 435f663..c65e4e4 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -2119,6 +2119,7 @@ remoteDispatchAuthList(virNetServerPtr server ATTRIBUTE_UNUSED,
uid_t callerUid;
gid_t callerGid;
pid_t callerPid;
+ unsigned long long timestamp;
/* If the client is root then we want to bypass the
* policykit auth to avoid root being denied if
@@ -2126,7 +2127,7 @@ remoteDispatchAuthList(virNetServerPtr server ATTRIBUTE_UNUSED,
*/
if (auth == VIR_NET_SERVER_SERVICE_AUTH_POLKIT) {
if (virNetServerClientGetUNIXIdentity(client, &callerUid, &callerGid,
- &callerPid) < 0) {
+ &callerPid, ×tamp) < 0) {
/* Don't do anything on error - it'll be validated at next
* phase of auth anyway */
virResetLastError();
@@ -2554,6 +2555,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED,
pid_t callerPid = -1;
gid_t callerGid = -1;
uid_t callerUid = -1;
+ unsigned long long timestamp;
const char *action;
int status = -1;
char *ident = NULL;
@@ -2579,7 +2581,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED,
}
if (virNetServerClientGetUNIXIdentity(client, &callerUid, &callerGid,
- &callerPid) < 0) {
+ &callerPid, ×tamp) < 0) {
goto authfail;
}
@@ -2587,7 +2589,11 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED,
(long long) callerPid, callerUid);
virCommandAddArg(cmd, "--process");
- virCommandAddArgFormat(cmd, "%lld", (long long) callerPid);
+ if (timestamp != 0) {
+ virCommandAddArgFormat(cmd, "%lld,%llu", (long long) callerPid,
timestamp);
+ } else {
+ virCommandAddArgFormat(cmd, "%lld", (long long) callerPid);
+ }
virCommandAddArg(cmd, "--allow-user-interaction");
if (virAsprintf(&ident, "pid:%lld,uid:%d",
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index 3838136..73aa28d 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -448,16 +448,20 @@ int virNetServerClientGetFD(virNetServerClientPtr client)
}
int virNetServerClientGetUNIXIdentity(virNetServerClientPtr client,
- uid_t *uid, gid_t *gid, pid_t *pid)
+ uid_t *uid, gid_t *gid, pid_t *pid,
+ unsigned long long *timestamp)
{
int ret = -1;
virNetServerClientLock(client);
if (client->sock)
- ret = virNetSocketGetUNIXIdentity(client->sock, uid, gid, pid);
+ ret = virNetSocketGetUNIXIdentity(client->sock,
+ uid, gid, pid,
+ timestamp);
virNetServerClientUnlock(client);
return ret;
}
+
bool virNetServerClientIsSecure(virNetServerClientPtr client)
{
bool secure = false;
diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h
index 154a160..9fc05c2 100644
--- a/src/rpc/virnetserverclient.h
+++ b/src/rpc/virnetserverclient.h
@@ -71,7 +71,8 @@ int virNetServerClientSetIdentity(virNetServerClientPtr client,
const char *virNetServerClientGetIdentity(virNetServerClientPtr client);
int virNetServerClientGetUNIXIdentity(virNetServerClientPtr client,
- uid_t *uid, gid_t *gid, pid_t *pid);
+ uid_t *uid, gid_t *gid, pid_t *pid,
+ unsigned long long *timestamp);
void virNetServerClientRef(virNetServerClientPtr client);
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index e82f375..1b4f894 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -828,31 +828,40 @@ int virNetSocketGetPort(virNetSocketPtr sock)
int virNetSocketGetUNIXIdentity(virNetSocketPtr sock,
uid_t *uid,
gid_t *gid,
- pid_t *pid)
+ pid_t *pid,
+ unsigned long long *timestamp)
{
struct ucred cr;
socklen_t cr_len = sizeof(cr);
+ int ret = -1;
+
virMutexLock(&sock->lock);
if (getsockopt(sock->fd, SOL_SOCKET, SO_PEERCRED, &cr, &cr_len) < 0) {
virReportSystemError(errno, "%s",
_("Failed to get client socket identity"));
- virMutexUnlock(&sock->lock);
- return -1;
+ goto cleanup;
}
+ if (virProcessGetStartTime(cr.pid, timestamp) < 0)
+ goto cleanup;
+
*pid = cr.pid;
*uid = cr.uid;
*gid = cr.gid;
+ ret = 0;
+
+cleanup:
virMutexUnlock(&sock->lock);
- return 0;
+ return ret;
}
#else
int virNetSocketGetUNIXIdentity(virNetSocketPtr sock ATTRIBUTE_UNUSED,
uid_t *uid ATTRIBUTE_UNUSED,
gid_t *gid ATTRIBUTE_UNUSED,
- pid_t *pid ATTRIBUTE_UNUSED)
+ pid_t *pid ATTRIBUTE_UNUSED,
+ unsigned long long *timestamp ATTRIBUTE_UNUSED)
{
/* XXX Many more OS support UNIX socket credentials we could port to. See dbus
....*/
virReportSystemError(ENOSYS, "%s",
diff --git a/src/rpc/virnetsocket.h b/src/rpc/virnetsocket.h
index 5ba7c8f..4eaf951 100644
--- a/src/rpc/virnetsocket.h
+++ b/src/rpc/virnetsocket.h
@@ -89,7 +89,8 @@ int virNetSocketGetPort(virNetSocketPtr sock);
int virNetSocketGetUNIXIdentity(virNetSocketPtr sock,
uid_t *uid,
gid_t *gid,
- pid_t *pid);
+ pid_t *pid,
+ unsigned long long *timestamp);
int virNetSocketSetBlocking(virNetSocketPtr sock,
bool blocking);
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index ac78f1d..f05f8f3 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -26,11 +26,19 @@
#include <errno.h>
#include <sys/wait.h>
+#ifdef __FreeBSD__
+# include <sys/param.h>
+# include <sys/sysctl.h>
+# include <sys/user.h>
+#endif
+
+#include "viratomic.h"
#include "virprocess.h"
#include "virterror_internal.h"
#include "memory.h"
#include "logging.h"
#include "util.h"
+#include "virstring.h"
#include "ignore-value.h"
#define virReportError(code, ...) \
virReportErrorHelper(VIR_FROM_NONE, code, __FILE__, \
@@ -239,3 +247,113 @@ int virProcessKill(pid_t pid, int sig)
return kill(pid, sig);
#endif
}
+
+
+#ifdef __linux__
+/*
+ * Port of code from polkitunixprocess.c under terms
+ * of the LGPLv2+
+ */
+int virProcessGetStartTime(pid_t pid,
+ unsigned long long *timestamp)
+{
+ char *filename = NULL;
+ char *buf = NULL;
+ char *tmp;
+ int ret = -1;
+ int len;
+ char **tokens = NULL;
+
+ if (virAsprintf(&filename, "/proc/%llu/stat",
+ (unsigned long long)pid) < 0) {
+ virReportOOMError();
+ return -1;
+ }
+
+ if ((len = virFileReadAll(filename, 1024, &buf)) < 0)
+ goto cleanup;
+
+ /* start time is the token at index 19 after the '(process name)' entry -
since only this
+ * field can contain the ')' character, search backwards for this to avoid
malicious
+ * processes trying to fool us
+ */
+
+ if (!(tmp = strrchr(buf, ')'))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot find start time in %s"),
+ filename);
+ goto cleanup;
+ }
+ tmp += 2; /* skip ') ' */
+ if ((tmp - buf) >= len) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot find start time in %s"),
+ filename);
+ goto cleanup;
+ }
+
+ tokens = virStringSplit(tmp, " ", 0);
+
+ if (virStringListLength(tokens) < 20) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot find start time in %s"),
+ filename);
+ goto cleanup;
+ }
+
+ if (virStrToLong_ull(tokens[19],
+ NULL,
+ 10,
+ timestamp) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot parse start time %s in %s"),
+ tokens[19], filename);
+ goto cleanup;
+ }
+
+ ret = 0;
+
+cleanup:
+ virStringFreeList(tokens);
+ VIR_FREE(filename);
+ VIR_FREE(buf);
+ return ret;
+}
+#elif defined(__FreeBSD__)
+int virProcessGetStartTime(pid_t pid,
+ unsigned long long *timestamp)
+{
+ struct kinfo_proc p;
+ int mib[4];
+ size_t len = 4;
+
+ sysctlnametomib("kern.proc.pid", mib, &len);
+
+ len = sizeof(struct kinfo_proc);
+ mib[3] = pid;
+
+ if (sysctl(mib, 4, &p, &len, NULL, 0) < 0) {
+ virReportSystemError(errno, "%s",
+ _("Unable to query process ID start time"));
+ return -1;
+ }
+
+ *timestamp = (unsigned long long)p.ki_start.tv_sec;
+
+ return 0;
+
+}
+#else
+int virProcessGetStartTime(pid_t pid,
+ unsigned long long *timestamp)
+{
+ static int warned = 0;
+ if (virAtomicIntInc(&warned) == 1) {
+ VIR_WARN("Process start time of pid %llu not available on this
platform",
+ (unsigned long long)pid);
+ warned = true;
+ }
+ *timestamp = 0;
+ return 0;
+}
+#endif
diff --git a/src/util/virprocess.h b/src/util/virprocess.h
index 048a73c..30ca21f 100644
--- a/src/util/virprocess.h
+++ b/src/util/virprocess.h
@@ -39,4 +39,7 @@ virProcessWait(pid_t pid, int *exitstatus)
int virProcessKill(pid_t pid, int sig);
+int virProcessGetStartTime(pid_t pid,
+ unsigned long long *timestamp);
+
#endif /* __VIR_PROCESS_H__ */
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 1917e9a..92289eb 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -166,3 +166,14 @@ void virStringFreeList(char **strings)
}
VIR_FREE(strings);
}
+
+
+size_t virStringListLength(char **strings)
+{
+ size_t i = 0;
+
+ while (strings && strings[i])
+ i++;
+
+ return i;
+}
diff --git a/src/util/virstring.h b/src/util/virstring.h
index a569fe0..d68ed2f 100644
--- a/src/util/virstring.h
+++ b/src/util/virstring.h
@@ -35,4 +35,6 @@ char *virStringJoin(const char **strings,
void virStringFreeList(char **strings);
+size_t virStringListLength(char **strings);
+
#endif /* __VIR_STRING_H__ */
--
1.8.3.1
7:22 a.m.
New subject: [libvirt] [v0.9.12-maint 7/9] Include process start time when doing polkit checks
On Wed, Sep 18, 2013 at 09:14:23PM -0600, Eric Blake wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Since PIDs can be reused, polkit prefers to be given
a (PID,start time) pair. If given a PID on its own,
it will attempt to lookup the start time in /proc/pid/stat,
though this is subject to races.
It is safer if the client app resolves the PID start
time itself, because as long as the app has the client
socket open, the client PID won't be reused.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
(cherry picked from commit 979e9c56a7aadf2dcfbddd1abfbad594b78b4468)
Signed-off-by: Eric Blake <eblake(a)redhat.com>
Conflicts:
src/libvirt_private.syms - not backported
src/locking/lock_daemon.c - not backported
src/rpc/virnetserverclient.c
src/rpc/virnetsocket.c
src/rpc/virnetsocket.h
src/util/viridentity.h - not backported
src/util/virprocess.c
src/util/virprocess.h
src/util/virstring.c
src/util/virstring.h
Most conflicts were contextual (this patch adds new functions,
but upstream intermediate patches not backported here also added
new features, and the resolution was picking out just the portions
needed by this commit). virnetsocket.c also had slightly
different locking semantics.
---
daemon/remote.c | 12 +++--
src/rpc/virnetserverclient.c | 8 ++-
src/rpc/virnetserverclient.h | 3 +-
src/rpc/virnetsocket.c | 19 +++++--
src/rpc/virnetsocket.h | 3 +-
src/util/virprocess.c | 118 +++++++++++++++++++++++++++++++++++++++++++
src/util/virprocess.h | 3 ++
src/util/virstring.c | 11 ++++
src/util/virstring.h | 2 +
9 files changed, 167 insertions(+), 12 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index 435f663..c65e4e4 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -2119,6 +2119,7 @@ remoteDispatchAuthList(virNetServerPtr server ATTRIBUTE_UNUSED,
uid_t callerUid;
gid_t callerGid;
pid_t callerPid;
+ unsigned long long timestamp;
/* If the client is root then we want to bypass the
* policykit auth to avoid root being denied if
@@ -2126,7 +2127,7 @@ remoteDispatchAuthList(virNetServerPtr server ATTRIBUTE_UNUSED,
*/
if (auth == VIR_NET_SERVER_SERVICE_AUTH_POLKIT) {
if (virNetServerClientGetUNIXIdentity(client, &callerUid, &callerGid,
- &callerPid) < 0) {
+ &callerPid, ×tamp) < 0) {
/* Don't do anything on error - it'll be validated at next
* phase of auth anyway */
virResetLastError();
@@ -2554,6 +2555,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED,
pid_t callerPid = -1;
gid_t callerGid = -1;
uid_t callerUid = -1;
+ unsigned long long timestamp;
const char *action;
int status = -1;
char *ident = NULL;
@@ -2579,7 +2581,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED,
}
if (virNetServerClientGetUNIXIdentity(client, &callerUid, &callerGid,
- &callerPid) < 0) {
+ &callerPid, ×tamp) < 0) {
goto authfail;
}
@@ -2587,7 +2589,11 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED,
(long long) callerPid, callerUid);
virCommandAddArg(cmd, "--process");
- virCommandAddArgFormat(cmd, "%lld", (long long) callerPid);
+ if (timestamp != 0) {
+ virCommandAddArgFormat(cmd, "%lld,%llu", (long long) callerPid,
timestamp);
+ } else {
+ virCommandAddArgFormat(cmd, "%lld", (long long) callerPid);
+ }
virCommandAddArg(cmd, "--allow-user-interaction");
if (virAsprintf(&ident, "pid:%lld,uid:%d",
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index 3838136..73aa28d 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -448,16 +448,20 @@ int virNetServerClientGetFD(virNetServerClientPtr client)
}
int virNetServerClientGetUNIXIdentity(virNetServerClientPtr client,
- uid_t *uid, gid_t *gid, pid_t *pid)
+ uid_t *uid, gid_t *gid, pid_t *pid,
+ unsigned long long *timestamp)
{
int ret = -1;
virNetServerClientLock(client);
if (client->sock)
- ret = virNetSocketGetUNIXIdentity(client->sock, uid, gid, pid);
+ ret = virNetSocketGetUNIXIdentity(client->sock,
+ uid, gid, pid,
+ timestamp);
virNetServerClientUnlock(client);
return ret;
}
+
bool virNetServerClientIsSecure(virNetServerClientPtr client)
{
bool secure = false;
diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h
index 154a160..9fc05c2 100644
--- a/src/rpc/virnetserverclient.h
+++ b/src/rpc/virnetserverclient.h
@@ -71,7 +71,8 @@ int virNetServerClientSetIdentity(virNetServerClientPtr client,
const char *virNetServerClientGetIdentity(virNetServerClientPtr client);
int virNetServerClientGetUNIXIdentity(virNetServerClientPtr client,
- uid_t *uid, gid_t *gid, pid_t *pid);
+ uid_t *uid, gid_t *gid, pid_t *pid,
+ unsigned long long *timestamp);
void virNetServerClientRef(virNetServerClientPtr client);
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index e82f375..1b4f894 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -828,31 +828,40 @@ int virNetSocketGetPort(virNetSocketPtr sock)
int virNetSocketGetUNIXIdentity(virNetSocketPtr sock,
uid_t *uid,
gid_t *gid,
- pid_t *pid)
+ pid_t *pid,
+ unsigned long long *timestamp)
{
struct ucred cr;
socklen_t cr_len = sizeof(cr);
+ int ret = -1;
+
virMutexLock(&sock->lock);
if (getsockopt(sock->fd, SOL_SOCKET, SO_PEERCRED, &cr, &cr_len) < 0)
{
virReportSystemError(errno, "%s",
_("Failed to get client socket identity"));
- virMutexUnlock(&sock->lock);
- return -1;
+ goto cleanup;
}
+ if (virProcessGetStartTime(cr.pid, timestamp) < 0)
+ goto cleanup;
+
*pid = cr.pid;
*uid = cr.uid;
*gid = cr.gid;
+ ret = 0;
+
+cleanup:
virMutexUnlock(&sock->lock);
- return 0;
+ return ret;
}
#else
int virNetSocketGetUNIXIdentity(virNetSocketPtr sock ATTRIBUTE_UNUSED,
uid_t *uid ATTRIBUTE_UNUSED,
gid_t *gid ATTRIBUTE_UNUSED,
- pid_t *pid ATTRIBUTE_UNUSED)
+ pid_t *pid ATTRIBUTE_UNUSED,
+ unsigned long long *timestamp ATTRIBUTE_UNUSED)
{
/* XXX Many more OS support UNIX socket credentials we could port to. See dbus
....*/
virReportSystemError(ENOSYS, "%s",
diff --git a/src/rpc/virnetsocket.h b/src/rpc/virnetsocket.h
index 5ba7c8f..4eaf951 100644
--- a/src/rpc/virnetsocket.h
+++ b/src/rpc/virnetsocket.h
@@ -89,7 +89,8 @@ int virNetSocketGetPort(virNetSocketPtr sock);
int virNetSocketGetUNIXIdentity(virNetSocketPtr sock,
uid_t *uid,
gid_t *gid,
- pid_t *pid);
+ pid_t *pid,
+ unsigned long long *timestamp);
int virNetSocketSetBlocking(virNetSocketPtr sock,
bool blocking);
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index ac78f1d..f05f8f3 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -26,11 +26,19 @@
#include <errno.h>
#include <sys/wait.h>
+#ifdef __FreeBSD__
+# include <sys/param.h>
+# include <sys/sysctl.h>
+# include <sys/user.h>
+#endif
+
+#include "viratomic.h"
#include "virprocess.h"
#include "virterror_internal.h"
#include "memory.h"
#include "logging.h"
#include "util.h"
+#include "virstring.h"
#include "ignore-value.h"
#define virReportError(code, ...) \
virReportErrorHelper(VIR_FROM_NONE, code, __FILE__, \
@@ -239,3 +247,113 @@ int virProcessKill(pid_t pid, int sig)
return kill(pid, sig);
#endif
}
+
+
+#ifdef __linux__
+/*
+ * Port of code from polkitunixprocess.c under terms
+ * of the LGPLv2+
+ */
+int virProcessGetStartTime(pid_t pid,
+ unsigned long long *timestamp)
+{
+ char *filename = NULL;
+ char *buf = NULL;
+ char *tmp;
+ int ret = -1;
+ int len;
+ char **tokens = NULL;
+
+ if (virAsprintf(&filename, "/proc/%llu/stat",
+ (unsigned long long)pid) < 0) {
+ virReportOOMError();
+ return -1;
+ }
+
+ if ((len = virFileReadAll(filename, 1024, &buf)) < 0)
+ goto cleanup;
+
+ /* start time is the token at index 19 after the '(process name)' entry -
since only this
+ * field can contain the ')' character, search backwards for this to avoid
malicious
+ * processes trying to fool us
+ */
+
+ if (!(tmp = strrchr(buf, ')'))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot find start time in %s"),
+ filename);
+ goto cleanup;
+ }
+ tmp += 2; /* skip ') ' */
+ if ((tmp - buf) >= len) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot find start time in %s"),
+ filename);
+ goto cleanup;
+ }
+
+ tokens = virStringSplit(tmp, " ", 0);
+
+ if (virStringListLength(tokens) < 20) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot find start time in %s"),
+ filename);
+ goto cleanup;
+ }
+
+ if (virStrToLong_ull(tokens[19],
+ NULL,
+ 10,
+ timestamp) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot parse start time %s in %s"),
+ tokens[19], filename);
+ goto cleanup;
+ }
+
+ ret = 0;
+
+cleanup:
+ virStringFreeList(tokens);
+ VIR_FREE(filename);
+ VIR_FREE(buf);
+ return ret;
+}
+#elif defined(__FreeBSD__)
+int virProcessGetStartTime(pid_t pid,
+ unsigned long long *timestamp)
+{
+ struct kinfo_proc p;
+ int mib[4];
+ size_t len = 4;
+
+ sysctlnametomib("kern.proc.pid", mib, &len);
+
+ len = sizeof(struct kinfo_proc);
+ mib[3] = pid;
+
+ if (sysctl(mib, 4, &p, &len, NULL, 0) < 0) {
+ virReportSystemError(errno, "%s",
+ _("Unable to query process ID start time"));
+ return -1;
+ }
+
+ *timestamp = (unsigned long long)p.ki_start.tv_sec;
+
+ return 0;
+
+}
+#else
+int virProcessGetStartTime(pid_t pid,
+ unsigned long long *timestamp)
+{
+ static int warned = 0;
+ if (virAtomicIntInc(&warned) == 1) {
+ VIR_WARN("Process start time of pid %llu not available on this
platform",
+ (unsigned long long)pid);
+ warned = true;
+ }
+ *timestamp = 0;
+ return 0;
+}
+#endif
diff --git a/src/util/virprocess.h b/src/util/virprocess.h
index 048a73c..30ca21f 100644
--- a/src/util/virprocess.h
+++ b/src/util/virprocess.h
@@ -39,4 +39,7 @@ virProcessWait(pid_t pid, int *exitstatus)
int virProcessKill(pid_t pid, int sig);
+int virProcessGetStartTime(pid_t pid,
+ unsigned long long *timestamp);
+
#endif /* __VIR_PROCESS_H__ */
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 1917e9a..92289eb 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -166,3 +166,14 @@ void virStringFreeList(char **strings)
}
VIR_FREE(strings);
}
+
+
+size_t virStringListLength(char **strings)
+{
+ size_t i = 0;
+
+ while (strings && strings[i])
+ i++;
+
+ return i;
+}
This looks a bit as if it could go into a separate commit since it adds
a new utility function, but that's minor. Otherwise ACK to the whole
series.
Cheers,
-- Guido
diff --git a/src/util/virstring.h b/src/util/virstring.h
index a569fe0..d68ed2f 100644
--- a/src/util/virstring.h
+++ b/src/util/virstring.h
@@ -35,4 +35,6 @@ char *virStringJoin(const char **strings,
void virStringFreeList(char **strings);
+size_t virStringListLength(char **strings);
+
#endif /* __VIR_STRING_H__ */
--
1.8.3.1
10:08 a.m.
New subject: [libvirt] [v0.9.12-maint 7/9] Include process start time when doing polkit checks
On 09/19/2013 06:22 AM, Guido Günther wrote:
On Wed, Sep 18, 2013 at 09:14:23PM -0600, Eric Blake wrote:
> From: "Daniel P. Berrange" <berrange(a)redhat.com>
>
> Since PIDs can be reused, polkit prefers to be given
> a (PID,start time) pair. If given a PID on its own,
> it will attempt to lookup the start time in /proc/pid/stat,
> though this is subject to races.
>
> It is safer if the client app resolves the PID start
> time itself, because as long as the app has the client
> socket open, the client PID won't be reused.
>
> Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
> (cherry picked from commit 979e9c56a7aadf2dcfbddd1abfbad594b78b4468)
> Signed-off-by: Eric Blake <eblake(a)redhat.com>
>
> +
> +
> +size_t virStringListLength(char **strings)
> +{
> + size_t i = 0;
> +
> + while (strings && strings[i])
> + i++;
> +
> + return i;
> +}
This looks a bit as if it could go into a separate commit since it adds
a new utility function, but that's minor. Otherwise ACK to the whole
series.
That utility function really was added during upstream commit 979e9c56
(yeah, we probably should have split that into two commits back in April
at the time of the upstream commit), but this backport is faithfully
reproducing what the original did.
Thanks for the review; I'll push the series shortly.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
10:14 p.m.
New subject: [libvirt] [v0.9.12-maint 8/9] Add support for using 3-arg pkcheck syntax for process (CVE-2013-4311)
From: "Daniel P. Berrange" <berrange(a)redhat.com>
With the existing pkcheck (pid, start time) tuple for identifying
the process, there is a race condition, where a process can make
a libvirt RPC call and in another thread exec a setuid application,
causing it to change to effective UID 0. This in turn causes polkit
to do its permission check based on the wrong UID.
To address this, libvirt must get the UID the caller had at time
of connect() (from SO_PEERCRED) and pass a (pid, start time, uid)
triple to the pkcheck program.
This fix requires that libvirt is re-built against a version of
polkit that has the fix for its CVE-2013-4288, so that libvirt
can see 'pkg-config --variable pkcheck_supports_uid polkit-gobject-1'
Signed-off-by: Colin Walters <walters(a)redhat.com>
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
(cherry picked from commit 922b7fda77b094dbf022d625238262ea05335666)
Signed-off-by: Eric Blake <eblake(a)redhat.com>
Conflicts:
configure.ac - context
libvirt.spec.in - context of indentation
src/access/viraccessdriverpolkit.c - not present on this branch
---
configure.ac | 8 ++++++++
daemon/remote.c | 22 +++++++++++++++++++---
libvirt.spec.in | 3 +--
3 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/configure.ac b/configure.ac
index fa9d537..73a335e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1160,6 +1160,14 @@ if test "x$with_polkit" = "xyes" || test
"x$with_polkit" = "xcheck"; then
AC_PATH_PROG([PKCHECK_PATH],[pkcheck], [], [/usr/sbin:$PATH])
if test "x$PKCHECK_PATH" != "x" ; then
AC_DEFINE_UNQUOTED([PKCHECK_PATH],["$PKCHECK_PATH"],[Location of pkcheck
program])
+ AC_MSG_CHECKING([whether pkcheck supports uid value])
+ pkcheck_supports_uid=`$PKG_CONFIG --variable pkcheck_supports_uid polkit-gobject-1`
+ if test "x$pkcheck_supports_uid" = "xtrue"; then
+ AC_MSG_RESULT([yes])
+ AC_DEFINE_UNQUOTED([PKCHECK_SUPPORTS_UID], 1, [Pass uid to pkcheck])
+ else
+ AC_MSG_RESULT([no])
+ fi
AC_DEFINE_UNQUOTED([HAVE_POLKIT], 1,
[use PolicyKit for UNIX socket access checks])
AC_DEFINE_UNQUOTED([HAVE_POLKIT1], 1,
diff --git a/daemon/remote.c b/daemon/remote.c
index c65e4e4..2cdb079 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -2560,10 +2560,12 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED,
int status = -1;
char *ident = NULL;
bool authdismissed = 0;
+ bool supportsuid = false;
char *pkout = NULL;
struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
virCommandPtr cmd = NULL;
+ static bool polkitInsecureWarned;
virMutexLock(&priv->lock);
action = virNetServerClientGetReadonly(client) ?
@@ -2585,14 +2587,28 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED,
goto authfail;
}
+ if (timestamp == 0) {
+ VIR_WARN("Failing polkit auth due to missing client (pid=%lld) start
time",
+ (long long)callerPid);
+ goto authfail;
+ }
+
VIR_INFO("Checking PID %lld running as %d",
(long long) callerPid, callerUid);
virCommandAddArg(cmd, "--process");
- if (timestamp != 0) {
- virCommandAddArgFormat(cmd, "%lld,%llu", (long long) callerPid,
timestamp);
+# ifdef PKCHECK_SUPPORTS_UID
+ supportsuid = true;
+# endif
+ if (supportsuid) {
+ virCommandAddArgFormat(cmd, "%lld,%llu,%lu",
+ (long long) callerPid, timestamp, (unsigned long)
callerUid);
} else {
- virCommandAddArgFormat(cmd, "%lld", (long long) callerPid);
+ if (!polkitInsecureWarned) {
+ VIR_WARN("No support for caller UID with pkcheck. This deployment is
known to be insecure.");
+ polkitInsecureWarned = true;
+ }
+ virCommandAddArgFormat(cmd, "%lld,%llu", (long long) callerPid,
timestamp);
}
virCommandAddArg(cmd, "--allow-user-interaction");
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 2e08abb..09272d4 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -364,8 +364,7 @@ BuildRequires: cyrus-sasl-devel
%endif
%if %{with_polkit}
%if 0%{?fedora} >= 12 || 0%{?rhel} >= 6
-# Only need the binary, not -devel
-BuildRequires: polkit >= 0.93
+BuildRequires: polkit-devel >= 0.93
%else
BuildRequires: PolicyKit-devel >= 0.6
%endif
--
1.8.3.1
10:14 p.m.
New subject: [libvirt] [v0.9.12-maint 9/9] Fix crash in remoteDispatchDomainMemoryStats (CVE-2013-4296)
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The 'stats' variable was not initialized to NULL, so if some
early validation of the RPC call fails, it is possible to jump
to the 'cleanup' label and VIR_FREE an uninitialized pointer.
This is a security flaw, since the API can be called from a
readonly connection which can trigger the validation checks.
This was introduced in release v0.9.1 onwards by
commit 158ba8730e44b7dd07a21ab90499996c5dec080a
Author: Daniel P. Berrange <berrange(a)redhat.com>
Date: Wed Apr 13 16:21:35 2011 +0100
Merge all returns paths from dispatcher into single path
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
(cherry picked from commit e7f400a110e2e3673b96518170bfea0855dd82c0)
Conflicts:
daemon/remote.c - context
---
daemon/remote.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index 2cdb079..f127eaa 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -1061,7 +1061,7 @@ remoteDispatchDomainMemoryStats(virNetServerPtr server
ATTRIBUTE_UNUSED,
remote_domain_memory_stats_ret *ret)
{
virDomainPtr dom = NULL;
- struct _virDomainMemoryStat *stats;
+ struct _virDomainMemoryStat *stats = NULL;
int nr_stats, i;
int rv = -1;
struct daemonClientPrivate *priv =
--
1.8.3.1
7:23 a.m.
On Wed, Sep 18, 2013 at 09:14:16PM -0600, Eric Blake wrote:
I've completed and pushed my backport work for both CVEs that
were patched today, into all branches v0.10.2-maint and newer
(basically, we have 0.10.2, then all releases since 1.0.2). One
last branch in active use (hello Debian) remains to be patched;
but here, the backport work had enough conflict resolutions that
I decided to post my work for review first.
I've been basing my patches for CVE-2013-4311 on Daniel's RHEL6 version
so far but this looks much nicer for v0.9.12-maint.
Cheers,
-- Guido
Daniel P. Berrange (9):
Introduce APIs for splitting/joining strings
Rename virKillProcess to virProcessKill
Rename virPid{Abort, Wait} to virProcess{Abort, Wait}
Rename virCommandTranslateStatus to virProcessTranslateStatus
Move virProcessKill into virprocess.{h, c}
Move virProcess{Kill, Abort, TranslateStatus} into virprocess.{c, h}
Include process start time when doing polkit checks
Add support for using 3-arg pkcheck syntax for process (CVE-2013-4311)
Fix crash in remoteDispatchDomainMemoryStats (CVE-2013-4296)
.gitignore | 1 +
configure.ac | 8 +
daemon/libvirtd.c | 3 +-
daemon/remote.c | 33 +++-
libvirt.spec.in | 3 +-
po/POTFILES.in | 1 +
src/Makefile.am | 2 +
src/libvirt_private.syms | 16 +-
src/lxc/lxc_container.c | 3 +-
src/lxc/lxc_controller.c | 3 +-
src/qemu/qemu_agent.c | 3 +-
src/qemu/qemu_monitor.c | 3 +-
src/qemu/qemu_process.c | 3 +-
src/rpc/virnetserverclient.c | 8 +-
src/rpc/virnetserverclient.h | 3 +-
src/rpc/virnetsocket.c | 22 ++-
src/rpc/virnetsocket.h | 3 +-
src/uml/uml_driver.c | 3 +-
src/util/command.c | 167 ++------------------
src/util/command.h | 8 -
src/util/util.c | 64 +-------
src/util/util.h | 1 -
src/util/virprocess.c | 359 +++++++++++++++++++++++++++++++++++++++++++
src/util/virprocess.h | 45 ++++++
src/util/virstring.c | 179 +++++++++++++++++++++
src/util/virstring.h | 40 +++++
tests/Makefile.am | 9 +-
tests/testutils.c | 5 +-
tests/virstringtest.c | 161 +++++++++++++++++++
29 files changed, 908 insertions(+), 251 deletions(-)
create mode 100644 src/util/virprocess.c
create mode 100644 src/util/virprocess.h
create mode 100644 src/util/virstring.c
create mode 100644 src/util/virstring.h
create mode 100644 tests/virstringtest.c
--
1.8.3.1
4082
days inactive
4082
days old
12 comments
2 participants
participants (2)
-
Eric Blake -
Guido Günther