From 3cf99237563e5321a3ac9d3c84963011b35201d5 Mon Sep 17 00:00:00 2001 From: CentOS Sources Date: Apr 27 2021 13:01:40 +0000 Subject: import libvirt-4.5.0-36.el7_9.5 --- diff --git a/SOURCES/libvirt-RHEL-virdevmapper-Don-t-leak-DIR-on-OOM-in-virDMSanitizepath.patch b/SOURCES/libvirt-RHEL-virdevmapper-Don-t-leak-DIR-on-OOM-in-virDMSanitizepath.patch new file mode 100644 index 0000000..582714f --- /dev/null +++ b/SOURCES/libvirt-RHEL-virdevmapper-Don-t-leak-DIR-on-OOM-in-virDMSanitizepath.patch @@ -0,0 +1,50 @@ +From 357fc40405c4967654972ecdf0b210fed885185d Mon Sep 17 00:00:00 2001 +Message-Id: <357fc40405c4967654972ecdf0b210fed885185d@dist-git> +From: Michal Privoznik +Date: Fri, 19 Mar 2021 11:51:58 +0100 +Subject: [PATCH] RHEL: virdevmapper: Don't leak DIR on OOM in + virDMSanitizepath() +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +This is a RHEL only patch, because it fixes a small issue in one +of backported commits that was basically rewritten from scratch +(v4.5.0-400-g6bf725c07c). + +During the rewrite I've introduced a problem, in which opened +/dev/mapper/ directory can leak if OOM happened afterwards. + +Fixes: 6bf725c07c25b56937c494b6c7e83e6ca27ec54c +Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1933557 +Signed-off-by: Michal Privoznik +Message-Id: +Reviewed-by: Ján Tomko +--- + src/util/virdevmapper.c | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) + +diff --git a/src/util/virdevmapper.c b/src/util/virdevmapper.c +index dcb3c08df5..33cde6ca0c 100644 +--- a/src/util/virdevmapper.c ++++ b/src/util/virdevmapper.c +@@ -216,7 +216,7 @@ virDMSanitizepath(const char *path) + VIR_AUTOFREE(char *) tmp = NULL; + + if (virAsprintf(&tmp, DEV_DM_DIR "/%s", ent->d_name) < 0) +- return NULL; ++ goto cleanup; + + if (stat(tmp, &sb[1]) == 0 && + sb[0].st_rdev == sb[1].st_rdev) { +@@ -225,6 +225,7 @@ virDMSanitizepath(const char *path) + } + } + ++ cleanup: + virDirClose(&dh); + return ret; + } +-- +2.31.0 + diff --git a/SOURCES/libvirt-virDevMapperGetTargetsImpl-Use-VIR_AUTOSTRINGLIST.patch b/SOURCES/libvirt-virDevMapperGetTargetsImpl-Use-VIR_AUTOSTRINGLIST.patch new file mode 100644 index 0000000..d4c155b --- /dev/null +++ b/SOURCES/libvirt-virDevMapperGetTargetsImpl-Use-VIR_AUTOSTRINGLIST.patch @@ -0,0 +1,79 @@ +From dce266130a7eef93d3f76940929cc92e1316b0a0 Mon Sep 17 00:00:00 2001 +Message-Id: +From: Michal Privoznik +Date: Mon, 8 Mar 2021 12:57:30 +0100 +Subject: [PATCH] virDevMapperGetTargetsImpl: Use VIR_AUTOSTRINGLIST +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Since we have VIR_AUTOSTRINGLIST we can use it to free string +lists used in the function automatically. + +Signed-off-by: Michal Privoznik +Reviewed-by: Daniel P. Berrangé +(cherry picked from commit b8ebbe05451fde7ce541564f73437a29ffd5db0d) + +Conflicts: +- src/util/virdevmapper.c: Another Glib conflict, but + fortunately, this time it's only in the context - the cherry + picked commit uses g_steal_pointer() because of + a3931b4996e46db409ad031e0230a507fb303287 + but the old code still uses VIR_STEAL_PTR(). + +Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1933557 +Signed-off-by: Michal Privoznik +Message-Id: +Reviewed-by: Ján Tomko +--- + src/util/virdevmapper.c | 16 +++------------- + 1 file changed, 3 insertions(+), 13 deletions(-) + +diff --git a/src/util/virdevmapper.c b/src/util/virdevmapper.c +index 6aee00112c..50d12ea0f1 100644 +--- a/src/util/virdevmapper.c ++++ b/src/util/virdevmapper.c +@@ -72,8 +72,7 @@ virDevMapperGetTargetsImpl(const char *path, + struct dm_task *dmt = NULL; + struct dm_deps *deps; + struct dm_info info; +- char **devPaths = NULL; +- char **recursiveDevPaths = NULL; ++ VIR_AUTOSTRINGLIST devPaths = NULL; + size_t i; + int ret = -1; + +@@ -139,28 +138,19 @@ virDevMapperGetTargetsImpl(const char *path, + goto cleanup; + } + +- recursiveDevPaths = NULL; + for (i = 0; i < deps->count; i++) { +- char **tmpPaths; ++ VIR_AUTOSTRINGLIST tmpPaths = NULL; + + if (virDevMapperGetTargetsImpl(devPaths[i], &tmpPaths, ttl - 1) < 0) + goto cleanup; + +- if (tmpPaths && +- virStringListMerge(&recursiveDevPaths, &tmpPaths) < 0) { +- virStringListFree(tmpPaths); ++ if (virStringListMerge(&devPaths, &tmpPaths) < 0) + goto cleanup; +- } + } + +- if (virStringListMerge(&devPaths, &recursiveDevPaths) < 0) +- goto cleanup; +- + VIR_STEAL_PTR(*devPaths_ret, devPaths); + ret = 0; + cleanup: +- virStringListFree(recursiveDevPaths); +- virStringListFree(devPaths); + dm_task_destroy(dmt); + return ret; + } +-- +2.31.0 + diff --git a/SOURCES/libvirt-virDevMapperGetTargetsImpl-Use-correct-length-when-copying-into-dm.name.patch b/SOURCES/libvirt-virDevMapperGetTargetsImpl-Use-correct-length-when-copying-into-dm.name.patch new file mode 100644 index 0000000..5eab1f5 --- /dev/null +++ b/SOURCES/libvirt-virDevMapperGetTargetsImpl-Use-correct-length-when-copying-into-dm.name.patch @@ -0,0 +1,63 @@ +From 575ba6e90f005cefb8b1100169a024083d8b4a4a Mon Sep 17 00:00:00 2001 +Message-Id: <575ba6e90f005cefb8b1100169a024083d8b4a4a@dist-git> +From: Michal Privoznik +Date: Mon, 8 Mar 2021 12:57:36 +0100 +Subject: [PATCH] virDevMapperGetTargetsImpl: Use correct length when copying + into dm.name +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +For reasons unknown, when rewriting this code and dropping +libdevmapper I've mistakenly used incorrect length of dm.name. In +linux/dm-ioctl.h the dm_ioctl struct is defined as follows: + + #define DM_NAME_LEN 128 + + struct dm_ioctl { + ... + char name[DM_NAME_LEN]; /* device name */ + ... + }; + +However, when copying string into this member, DM_TABLE_DEPS was +used, which is defined as follows: + + #define DM_TABLE_DEPS _IOWR(DM_IOCTL, DM_TABLE_DEPS_CMD, struct dm_ioctl) + +After decryption, this results in the following size: 3241737483. + +Fixes: 22494556542c676d1b9e7f1c1f2ea13ac17e1e3e +Signed-off-by: Michal Privoznik +Reviewed-by: Daniel P. Berrangé +(cherry picked from commit 4f30c1bb8c32374c62f90abb5aa224611c59c363) + +Conflicts: +- src/util/virdevmapper.c: The same story as a few commits ago: + virStrcpy() returns a pointer, but the cherry picked commit + expects integer. + +Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1933557 +Signed-off-by: Michal Privoznik +Message-Id: +Reviewed-by: Ján Tomko +--- + src/util/virdevmapper.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/util/virdevmapper.c b/src/util/virdevmapper.c +index 4994b4caef..dcb3c08df5 100644 +--- a/src/util/virdevmapper.c ++++ b/src/util/virdevmapper.c +@@ -257,7 +257,7 @@ virDevMapperGetTargetsImpl(int controlFD, + if (!(sanitizedPath = virDMSanitizepath(path))) + return 0; + +- if (virStrcpy(dm.name, sanitizedPath, DM_TABLE_DEPS) == NULL) { ++ if (virStrcpy(dm.name, sanitizedPath, DM_NAME_LEN) == NULL) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Resolved device mapper name too long")); + return -1; +-- +2.31.0 + diff --git a/SOURCES/libvirt-virdevmapper-Don-t-cache-device-mapper-major.patch b/SOURCES/libvirt-virdevmapper-Don-t-cache-device-mapper-major.patch new file mode 100644 index 0000000..a63c189 --- /dev/null +++ b/SOURCES/libvirt-virdevmapper-Don-t-cache-device-mapper-major.patch @@ -0,0 +1,107 @@ +From fb8c44edabd188957fe3e3deb97a605f71c394b8 Mon Sep 17 00:00:00 2001 +Message-Id: +From: Michal Privoznik +Date: Mon, 8 Mar 2021 12:57:32 +0100 +Subject: [PATCH] virdevmapper: Don't cache device-mapper major +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The device mapper major is needed in virIsDevMapperDevice() which +determines whether given device is managed by device-mapper. This +number is obtained by parsing /proc/devices and then stored in a +global variable so that the file doesn't have to be parsed again. +However, as it turns out this logic is flawed - the major number +is not static and can change as it can be specified as a +parameter when loading the dm-mod module. + +Unfortunately, I was not able to come up with a good solution and +thus the /proc/devices file is being parsed every time we need +the device mapper major. + +Signed-off-by: Michal Privoznik +Reviewed-by: Peter Krempa +Reviewed-by: Christian Ehrhardt +Tested-by: Christian Ehrhardt +(cherry picked from commit 82bb167f0d15b733b23931205be3488b83cb9ec6) + +Conflicts: +- src/util/virdevmapper.c: The VIR_ONCE_GLOBAL_INIT() call was + missing a semicolon at EOL which was added there in + af36f8a641809556ac18dcc076f996033cb2385c which isn't + backported. + +Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1933557 +Signed-off-by: Michal Privoznik +Message-Id: <206f7453debcfd7a013317989893be8a0688045c.1615203117.git.mprivozn@redhat.com> +Reviewed-by: Ján Tomko +--- + src/util/virdevmapper.c | 17 +++++------------ + 1 file changed, 5 insertions(+), 12 deletions(-) + +diff --git a/src/util/virdevmapper.c b/src/util/virdevmapper.c +index ebf446d966..725de736c1 100644 +--- a/src/util/virdevmapper.c ++++ b/src/util/virdevmapper.c +@@ -49,11 +49,9 @@ + + verify(BUF_SIZE > sizeof(struct dm_ioctl)); + +-static unsigned int virDMMajor; +- + + static int +-virDevMapperOnceInit(void) ++virDevMapperGetMajor(unsigned int *major) + { + VIR_AUTOFREE(char *) buf = NULL; + VIR_AUTOSTRINGLIST lines = NULL; +@@ -72,7 +70,7 @@ virDevMapperOnceInit(void) + + if (sscanf(lines[i], "%u %ms\n", &maj, &dev) == 2 && + STREQ(dev, DM_NAME)) { +- virDMMajor = maj; ++ *major = maj; + break; + } + } +@@ -88,9 +86,6 @@ virDevMapperOnceInit(void) + } + + +-VIR_ONCE_GLOBAL_INIT(virDevMapper) +- +- + static void * + virDMIoctl(int controlFD, int cmd, struct dm_ioctl *dm, char **buf) + { +@@ -318,9 +313,6 @@ virDevMapperGetTargets(const char *path, + * consist of devices or yet another targets. If that's the + * case, we have to stop recursion somewhere. */ + +- if (virDevMapperInitialize() < 0) +- return -1; +- + if ((controlFD = virDMOpen()) < 0) + return -1; + +@@ -332,13 +324,14 @@ bool + virIsDevMapperDevice(const char *dev_name) + { + struct stat buf; ++ unsigned int major; + +- if (virDevMapperInitialize() < 0) ++ if (virDevMapperGetMajor(&major) < 0) + return false; + + if (!stat(dev_name, &buf) && + S_ISBLK(buf.st_mode) && +- major(buf.st_rdev) == virDMMajor) ++ major(buf.st_rdev) == major) + return true; + + return false; +-- +2.31.0 + diff --git a/SOURCES/libvirt-virdevmapper-Don-t-use-libdevmapper-to-obtain-dependencies.patch b/SOURCES/libvirt-virdevmapper-Don-t-use-libdevmapper-to-obtain-dependencies.patch new file mode 100644 index 0000000..97f30bb --- /dev/null +++ b/SOURCES/libvirt-virdevmapper-Don-t-use-libdevmapper-to-obtain-dependencies.patch @@ -0,0 +1,496 @@ +From 6bf725c07c25b56937c494b6c7e83e6ca27ec54c Mon Sep 17 00:00:00 2001 +Message-Id: <6bf725c07c25b56937c494b6c7e83e6ca27ec54c@dist-git> +From: Michal Privoznik +Date: Mon, 8 Mar 2021 12:57:31 +0100 +Subject: [PATCH] virdevmapper: Don't use libdevmapper to obtain dependencies +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +CVE-2020-14339 + +When building domain's private /dev in a namespace, libdevmapper +is consulted for getting full dependency tree of domain's disks. +The reason is that for a multipath devices all dependent devices +must be created in the namespace and allowed in CGroups. + +However, this approach is very fragile as building of namespace +happens in the forked off child process, after mass close of FDs +and just before dropping privileges and execing QEMU. And it so +happens that when calling libdevmapper APIs, one of them opens +/dev/mapper/control and saves the FD into a global variable. The +FD is kept open until the lib is unlinked or dm_lib_release() is +called explicitly. We are doing neither. + +However, the virDevMapperGetTargets() function is called also +from libvirtd (when setting up CGroups) and thus has to be thread +safe. Unfortunately, libdevmapper APIs are not thread safe (nor +async signal safe) and thus we can't use them. Reimplement what +libdevmapper would do using plain C (ioctl()-s, /proc/devices +parsing, /dev/mapper dirwalking, and so on). + +Fixes: a30078cb832646177defd256e77c632905f1e6d0 +Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1858260 + +Signed-off-by: Michal Privoznik +Reviewed-by: Daniel P. Berrangé +(cherry picked from commit 22494556542c676d1b9e7f1c1f2ea13ac17e1e3e) + +Conflicts: +- po/POTFILES.in: This file doesn't exist, only POTFILES does. +- src/util/virdevmapper.c: Couple of problems. Firstly, + 03c532cf9711dd6ad35380455a77141ef7d492ab isn't backported, so + the header files include looks a bit different. Secondly, + 91d88aaf23994691ea94973dd988ca5825c1e1b1 isn't backported, so + we have to stick with virAsprintf() instead of + g_strdup_printf(). Thirdly, virStrncpy() behaves a bit + differently: with the old code it returns pointer to copied + string or NULL, with the new code (cherry-picked one) it + already returns an integer, and on the top of that, specifying + -1 for the source len means strlen() is used under the hood + (see 6c0d0210cbcd5d647f0d882c07f077d444bc707d and + 7d70a63b947e9a654a4e3fffa0ffa355f5549ec7 - neither is + backported). + +In addition, I had to de-GLib the code. In RHEL-7 there is no +g_autofree, G_STATIC_ASSERT, g_new0, g_strdup, g_strdup_printf, +or g_steal_pointer. I had switch these to their old counterparts +we used to use. + +Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1933557 +Signed-off-by: Michal Privoznik +Message-Id: <3c7fd3071d8a9a58c8716f850880e3a0869243d3.1615203117.git.mprivozn@redhat.com> +Reviewed-by: Ján Tomko +--- + po/POTFILES | 1 + + src/util/virdevmapper.c | 315 +++++++++++++++++++++++++++++----------- + 2 files changed, 228 insertions(+), 88 deletions(-) + +diff --git a/po/POTFILES b/po/POTFILES +index be2874487c..82a08da04a 100644 +--- a/po/POTFILES ++++ b/po/POTFILES +@@ -206,6 +206,7 @@ src/util/vircommand.c + src/util/virconf.c + src/util/vircrypto.c + src/util/virdbus.c ++src/util/virdevmapper.c + src/util/virdnsmasq.c + src/util/virerror.c + src/util/virerror.h +diff --git a/src/util/virdevmapper.c b/src/util/virdevmapper.c +index 50d12ea0f1..ebf446d966 100644 +--- a/src/util/virdevmapper.c ++++ b/src/util/virdevmapper.c +@@ -23,40 +23,67 @@ + + #include + +-#ifdef MAJOR_IN_MKDEV +-# include +-#elif MAJOR_IN_SYSMACROS ++#include "virdevmapper.h" ++#include "internal.h" ++ ++#ifdef __linux__ + # include +-#endif ++# include ++# include ++# include ++# include ++# include + +-#ifdef WITH_DEVMAPPER +-# include +-#endif ++# include "virthread.h" ++# include "viralloc.h" ++# include "virstring.h" ++# include "virfile.h" ++ ++# define VIR_FROM_THIS VIR_FROM_STORAGE ++ ++# define PROC_DEVICES "/proc/devices" ++# define DM_NAME "device-mapper" ++# define DEV_DM_DIR "/dev/" DM_DIR ++# define CONTROL_PATH DEV_DM_DIR "/" DM_CONTROL_NODE ++# define BUF_SIZE (16 * 1024) ++ ++verify(BUF_SIZE > sizeof(struct dm_ioctl)); ++ ++static unsigned int virDMMajor; + +-#include "virdevmapper.h" +-#include "internal.h" +-#include "virthread.h" +-#include "viralloc.h" +-#include "virstring.h" +- +-#ifdef WITH_DEVMAPPER +-static void +-virDevMapperDummyLogger(int level ATTRIBUTE_UNUSED, +- const char *file ATTRIBUTE_UNUSED, +- int line ATTRIBUTE_UNUSED, +- int dm_errno ATTRIBUTE_UNUSED, +- const char *fmt ATTRIBUTE_UNUSED, +- ...) +-{ +- return; +-} + + static int + virDevMapperOnceInit(void) + { +- /* Ideally, we would not need this. But libdevmapper prints +- * error messages to stderr by default. Sad but true. */ +- dm_log_with_errno_init(virDevMapperDummyLogger); ++ VIR_AUTOFREE(char *) buf = NULL; ++ VIR_AUTOSTRINGLIST lines = NULL; ++ size_t i; ++ ++ if (virFileReadAll(PROC_DEVICES, BUF_SIZE, &buf) < 0) ++ return -1; ++ ++ lines = virStringSplit(buf, "\n", 0); ++ if (!lines) ++ return -1; ++ ++ for (i = 0; lines[i]; i++) { ++ VIR_AUTOFREE(char *) dev = NULL; ++ unsigned int maj; ++ ++ if (sscanf(lines[i], "%u %ms\n", &maj, &dev) == 2 && ++ STREQ(dev, DM_NAME)) { ++ virDMMajor = maj; ++ break; ++ } ++ } ++ ++ if (!lines[i]) { ++ virReportError(VIR_ERR_INTERNAL_ERROR, ++ _("Unable to find major for %s"), ++ DM_NAME); ++ return -1; ++ } ++ + return 0; + } + +@@ -64,95 +91,200 @@ virDevMapperOnceInit(void) + VIR_ONCE_GLOBAL_INIT(virDevMapper) + + ++static void * ++virDMIoctl(int controlFD, int cmd, struct dm_ioctl *dm, char **buf) ++{ ++ size_t bufsize = BUF_SIZE; ++ ++ reread: ++ if (VIR_ALLOC_N(*buf, bufsize) < 0) ++ return NULL; ++ ++ dm->version[0] = DM_VERSION_MAJOR; ++ dm->version[1] = 0; ++ dm->version[2] = 0; ++ dm->data_size = bufsize; ++ dm->data_start = sizeof(struct dm_ioctl); ++ ++ memcpy(*buf, dm, sizeof(struct dm_ioctl)); ++ ++ if (ioctl(controlFD, cmd, *buf) < 0) { ++ VIR_FREE(*buf); ++ return NULL; ++ } ++ ++ memcpy(dm, *buf, sizeof(struct dm_ioctl)); ++ ++ if (dm->flags & DM_BUFFER_FULL_FLAG) { ++ bufsize += BUF_SIZE; ++ VIR_FREE(*buf); ++ goto reread; ++ } ++ ++ return *buf + dm->data_start; ++} ++ ++ + static int +-virDevMapperGetTargetsImpl(const char *path, +- char ***devPaths_ret, +- unsigned int ttl) ++virDMOpen(void) + { +- struct dm_task *dmt = NULL; +- struct dm_deps *deps; +- struct dm_info info; +- VIR_AUTOSTRINGLIST devPaths = NULL; +- size_t i; +- int ret = -1; ++ VIR_AUTOCLOSE controlFD = -1; ++ struct dm_ioctl dm; ++ VIR_AUTOFREE(char *) tmp = NULL; ++ int ret; + +- *devPaths_ret = NULL; ++ memset(&dm, 0, sizeof(dm)); + +- if (virDevMapperInitialize() < 0) +- return ret; ++ if ((controlFD = open(CONTROL_PATH, O_RDWR)) < 0) ++ return -1; + +- if (ttl == 0) { +- errno = ELOOP; ++ if (!virDMIoctl(controlFD, DM_VERSION, &dm, &tmp)) { ++ virReportSystemError(errno, "%s", ++ _("Unable to get device-mapper version")); ++ return -1; ++ } ++ ++ if (dm.version[0] != DM_VERSION_MAJOR) { ++ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, ++ _("Unsupported device-mapper version. Expected %d got %d"), ++ DM_VERSION_MAJOR, dm.version[0]); ++ return -1; ++ } ++ ++ ret = controlFD; ++ controlFD = -1; ++ return ret; ++} ++ ++ ++static char * ++virDMSanitizepath(const char *path) ++{ ++ VIR_AUTOFREE(char *) dmDirPath = NULL; ++ struct dirent *ent = NULL; ++ struct stat sb[2]; ++ DIR *dh = NULL; ++ const char *p; ++ char *ret = NULL; ++ int rc; ++ ++ /* If a path is NOT provided then assume it's DM name */ ++ p = strrchr(path, '/'); ++ ++ if (!p) { ++ ignore_value(VIR_STRDUP(ret, path)); + return ret; ++ } else { ++ p++; + } + +- if (!virIsDevMapperDevice(path)) +- return 0; ++ /* It's a path. Check if the last component is DM name */ ++ if (stat(path, &sb[0]) < 0) { ++ virReportError(errno, ++ _("Unable to stat %p"), ++ path); ++ return NULL; ++ } + +- if (!(dmt = dm_task_create(DM_DEVICE_DEPS))) { +- if (errno == ENOENT || errno == ENODEV) { +- /* It's okay. Kernel is probably built without +- * devmapper support. */ +- ret = 0; +- } ++ if (virAsprintf(&dmDirPath, DEV_DM_DIR "/%s", p) < 0) ++ return NULL; ++ ++ if (stat(dmDirPath, &sb[1]) == 0 && ++ sb[0].st_rdev == sb[1].st_rdev) { ++ ignore_value(VIR_STRDUP(ret, p)); + return ret; + } + +- if (!dm_task_set_name(dmt, path)) { +- if (errno == ENOENT) { +- /* It's okay, @path is not managed by devmapper => +- * not a devmapper device. */ +- ret = 0; ++ /* The last component of @path wasn't DM name. Let's check if ++ * there's a device under /dev/mapper/ with the same rdev. */ ++ if (virDirOpen(&dh, DEV_DM_DIR) < 0) ++ return NULL; ++ ++ while ((rc = virDirRead(dh, &ent, DEV_DM_DIR)) > 0) { ++ VIR_AUTOFREE(char *) tmp = NULL; ++ ++ if (virAsprintf(&tmp, DEV_DM_DIR "/%s", ent->d_name) < 0) ++ return NULL; ++ ++ if (stat(tmp, &sb[1]) == 0 && ++ sb[0].st_rdev == sb[0].st_rdev) { ++ VIR_STEAL_PTR(ret, tmp); ++ break; + } +- goto cleanup; + } + +- dm_task_no_open_count(dmt); ++ virDirClose(&dh); ++ return ret; ++} + +- if (!dm_task_run(dmt)) { +- if (errno == ENXIO) { +- /* If @path = "/dev/mapper/control" ENXIO is returned. */ +- ret = 0; +- } +- goto cleanup; ++ ++static int ++virDevMapperGetTargetsImpl(int controlFD, ++ const char *path, ++ char ***devPaths_ret, ++ unsigned int ttl) ++{ ++ VIR_AUTOFREE(char *) sanitizedPath = NULL; ++ VIR_AUTOFREE(char *) buf = NULL; ++ struct dm_ioctl dm; ++ struct dm_target_deps *deps = NULL; ++ VIR_AUTOSTRINGLIST devPaths = NULL; ++ size_t i; ++ ++ memset(&dm, 0, sizeof(dm)); ++ *devPaths_ret = NULL; ++ ++ if (ttl == 0) { ++ errno = ELOOP; ++ return -1; + } + +- if (!dm_task_get_info(dmt, &info)) +- goto cleanup; ++ if (!virIsDevMapperDevice(path)) ++ return 0; ++ ++ if (!(sanitizedPath = virDMSanitizepath(path))) ++ return 0; + +- if (!info.exists) { +- ret = 0; +- goto cleanup; ++ if (virStrcpy(dm.name, sanitizedPath, DM_TABLE_DEPS) == NULL) { ++ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", ++ _("Resolved device mapper name too long")); ++ return -1; + } + +- if (!(deps = dm_task_get_deps(dmt))) +- goto cleanup; ++ deps = virDMIoctl(controlFD, DM_TABLE_DEPS, &dm, &buf); ++ if (!deps) { ++ if (errno == ENXIO) ++ return 0; ++ ++ virReportSystemError(errno, ++ _("Unable to query dependencies for %s"), ++ path); ++ return -1; ++ } + + if (VIR_ALLOC_N_QUIET(devPaths, deps->count + 1) < 0) +- goto cleanup; ++ return -1; + + for (i = 0; i < deps->count; i++) { + if (virAsprintfQuiet(&devPaths[i], "/dev/block/%u:%u", +- major(deps->device[i]), +- minor(deps->device[i])) < 0) +- goto cleanup; ++ major(deps->dev[i]), ++ minor(deps->dev[i])) < 0) { ++ return -1; ++ } + } + + for (i = 0; i < deps->count; i++) { + VIR_AUTOSTRINGLIST tmpPaths = NULL; + +- if (virDevMapperGetTargetsImpl(devPaths[i], &tmpPaths, ttl - 1) < 0) +- goto cleanup; ++ if (virDevMapperGetTargetsImpl(controlFD, devPaths[i], &tmpPaths, ttl - 1) < 0) ++ return -1; + + if (virStringListMerge(&devPaths, &tmpPaths) < 0) +- goto cleanup; ++ return -1; + } + + VIR_STEAL_PTR(*devPaths_ret, devPaths); +- ret = 0; +- cleanup: +- dm_task_destroy(dmt); +- return ret; ++ return 0; + } + + +@@ -171,9 +303,6 @@ virDevMapperGetTargetsImpl(const char *path, + * If @path consists of yet another devmapper targets these are + * consulted recursively. + * +- * If we don't have permissions to talk to kernel, -1 is returned +- * and errno is set to EBADF. +- * + * Returns 0 on success, + * -1 otherwise (with errno set, no libvirt error is + * reported) +@@ -182,13 +311,20 @@ int + virDevMapperGetTargets(const char *path, + char ***devPaths) + { ++ VIR_AUTOCLOSE controlFD = -1; + const unsigned int ttl = 32; + + /* Arbitrary limit on recursion level. A devmapper target can + * consist of devices or yet another targets. If that's the + * case, we have to stop recursion somewhere. */ + +- return virDevMapperGetTargetsImpl(path, devPaths, ttl); ++ if (virDevMapperInitialize() < 0) ++ return -1; ++ ++ if ((controlFD = virDMOpen()) < 0) ++ return -1; ++ ++ return virDevMapperGetTargetsImpl(controlFD, path, devPaths, ttl); + } + + +@@ -197,15 +333,18 @@ virIsDevMapperDevice(const char *dev_name) + { + struct stat buf; + ++ if (virDevMapperInitialize() < 0) ++ return false; ++ + if (!stat(dev_name, &buf) && + S_ISBLK(buf.st_mode) && +- dm_is_dm_major(major(buf.st_rdev))) +- return true; ++ major(buf.st_rdev) == virDMMajor) ++ return true; + + return false; + } + +-#else /* ! WITH_DEVMAPPER */ ++#else /* !defined(__linux__) */ + + int + virDevMapperGetTargets(const char *path ATTRIBUTE_UNUSED, +@@ -221,4 +360,4 @@ virIsDevMapperDevice(const char *dev_name G_GNUC_UNUSED) + { + return false; + } +-#endif /* ! WITH_DEVMAPPER */ ++#endif /* ! defined(__linux__) */ +-- +2.31.0 + diff --git a/SOURCES/libvirt-virdevmapper-Handle-kernel-without-device-mapper-support.patch b/SOURCES/libvirt-virdevmapper-Handle-kernel-without-device-mapper-support.patch new file mode 100644 index 0000000..84bd3fa --- /dev/null +++ b/SOURCES/libvirt-virdevmapper-Handle-kernel-without-device-mapper-support.patch @@ -0,0 +1,88 @@ +From 8474f0f611f4372bd4f98c6df92f50566f631db0 Mon Sep 17 00:00:00 2001 +Message-Id: <8474f0f611f4372bd4f98c6df92f50566f631db0@dist-git> +From: Michal Privoznik +Date: Mon, 8 Mar 2021 12:57:33 +0100 +Subject: [PATCH] virdevmapper: Handle kernel without device-mapper support +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +In one of my latest patch (v6.6.0~30) I was trying to remove +libdevmapper use in favor of our own implementation. However, the +code did not take into account that device mapper can be not +compiled into the kernel (e.g. be a separate module that's not +loaded) in which case /proc/devices won't have the device-mapper +major number and thus virDevMapperGetTargets() and/or +virIsDevMapperDevice() fails. + +However, such failure is safe to ignore, because if device mapper +is missing then there can't be any multipath devices and thus we +don't need to allow the deps in CGroups, nor create them in the +domain private namespace, etc. + +Fixes: 22494556542c676d1b9e7f1c1f2ea13ac17e1e3e +Reported-by: Andrea Bolognani +Reported-by: Christian Ehrhardt +Signed-off-by: Michal Privoznik +Reviewed-by: Peter Krempa +Reviewed-by: Christian Ehrhardt +Tested-by: Christian Ehrhardt +(cherry picked from commit feb8564a3cc63bc8f68284063d53ec0d2d81a1cc) +Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1933557 +Signed-off-by: Michal Privoznik +Message-Id: <11f41c0fd7b7c8365e24398173cb0bb2f8e785de.1615203117.git.mprivozn@redhat.com> +Reviewed-by: Ján Tomko +--- + src/util/virdevmapper.c | 20 ++++++++++++++++++-- + 1 file changed, 18 insertions(+), 2 deletions(-) + +diff --git a/src/util/virdevmapper.c b/src/util/virdevmapper.c +index 725de736c1..a76fe95922 100644 +--- a/src/util/virdevmapper.c ++++ b/src/util/virdevmapper.c +@@ -57,6 +57,9 @@ virDevMapperGetMajor(unsigned int *major) + VIR_AUTOSTRINGLIST lines = NULL; + size_t i; + ++ if (!virFileExists(CONTROL_PATH)) ++ return -2; ++ + if (virFileReadAll(PROC_DEVICES, BUF_SIZE, &buf) < 0) + return -1; + +@@ -130,8 +133,13 @@ virDMOpen(void) + + memset(&dm, 0, sizeof(dm)); + +- if ((controlFD = open(CONTROL_PATH, O_RDWR)) < 0) ++ if ((controlFD = open(CONTROL_PATH, O_RDWR)) < 0) { ++ if (errno == ENOENT) ++ return -2; ++ ++ virReportSystemError(errno, _("Unable to open %s"), CONTROL_PATH); + return -1; ++ } + + if (!virDMIoctl(controlFD, DM_VERSION, &dm, &tmp)) { + virReportSystemError(errno, "%s", +@@ -313,8 +321,16 @@ virDevMapperGetTargets(const char *path, + * consist of devices or yet another targets. If that's the + * case, we have to stop recursion somewhere. */ + +- if ((controlFD = virDMOpen()) < 0) ++ if ((controlFD = virDMOpen()) < 0) { ++ if (controlFD == -2) { ++ /* The CONTROL_PATH doesn't exist. Probably the ++ * module isn't loaded, yet. Don't error out, just ++ * exit. */ ++ return 0; ++ } ++ + return -1; ++ } + + return virDevMapperGetTargetsImpl(controlFD, path, devPaths, ttl); + } +-- +2.31.0 + diff --git a/SOURCES/libvirt-virdevmapper-Ignore-all-errors-when-opening-dev-mapper-control.patch b/SOURCES/libvirt-virdevmapper-Ignore-all-errors-when-opening-dev-mapper-control.patch new file mode 100644 index 0000000..d5b8c7c --- /dev/null +++ b/SOURCES/libvirt-virdevmapper-Ignore-all-errors-when-opening-dev-mapper-control.patch @@ -0,0 +1,95 @@ +From fb3d8e295473034a271f09c6ce46f9500fcfa2c5 Mon Sep 17 00:00:00 2001 +Message-Id: +From: Michal Privoznik +Date: Mon, 8 Mar 2021 12:57:34 +0100 +Subject: [PATCH] virdevmapper: Ignore all errors when opening + /dev/mapper/control +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +So far, only ENOENT is ignored (to deal with kernels without +devmapper). However, as reported on the list, under certain +scenarios a different error can occur. For instance, when libvirt +is running inside a container which doesn't have permissions to +talk to the devmapper. If this is the case, then open() returns +-1 and sets errno=EPERM. + +Assuming that multipath devices are fairly narrow use case and +using them in a restricted container is even more narrow the best +fix seems to be to ignore all open errors BUT produce a warning +on failure. To avoid flooding logs with warnings on kernels +without devmapper the level is reduced to a plain debug message. + +Reported-by: Christian Ehrhardt +Reviewed-by: Christian Ehrhardt +Signed-off-by: Michal Privoznik +(cherry picked from commit 53d9af1e7924757e3b5f661131dd707d7110d094) + +I had to replace g_strerror() from the original commit with +virStrerror() which is what we use in downstream. + +Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1933557 +Signed-off-by: Michal Privoznik +Message-Id: <8e45903b261c50d92cea4bf3d87f724a262c928f.1615203117.git.mprivozn@redhat.com> +Reviewed-by: Ján Tomko +--- + src/util/virdevmapper.c | 25 +++++++++++++++++-------- + 1 file changed, 17 insertions(+), 8 deletions(-) + +diff --git a/src/util/virdevmapper.c b/src/util/virdevmapper.c +index a76fe95922..a04d9650a6 100644 +--- a/src/util/virdevmapper.c ++++ b/src/util/virdevmapper.c +@@ -38,9 +38,12 @@ + # include "viralloc.h" + # include "virstring.h" + # include "virfile.h" ++# include "virlog.h" + + # define VIR_FROM_THIS VIR_FROM_STORAGE + ++VIR_LOG_INIT("util.virdevmapper"); ++ + # define PROC_DEVICES "/proc/devices" + # define DM_NAME "device-mapper" + # define DEV_DM_DIR "/dev/" DM_DIR +@@ -134,11 +137,17 @@ virDMOpen(void) + memset(&dm, 0, sizeof(dm)); + + if ((controlFD = open(CONTROL_PATH, O_RDWR)) < 0) { +- if (errno == ENOENT) +- return -2; +- +- virReportSystemError(errno, _("Unable to open %s"), CONTROL_PATH); +- return -1; ++ char ebuf[1024]; ++ ++ /* We can't talk to devmapper. Produce a warning and let ++ * the caller decide what to do next. */ ++ if (errno == ENOENT) { ++ VIR_DEBUG("device mapper not available"); ++ } else { ++ VIR_WARN("unable to open %s: %s", ++ CONTROL_PATH, virStrerror(errno, ebuf, sizeof(ebuf))); ++ } ++ return -2; + } + + if (!virDMIoctl(controlFD, DM_VERSION, &dm, &tmp)) { +@@ -323,9 +332,9 @@ virDevMapperGetTargets(const char *path, + + if ((controlFD = virDMOpen()) < 0) { + if (controlFD == -2) { +- /* The CONTROL_PATH doesn't exist. Probably the +- * module isn't loaded, yet. Don't error out, just +- * exit. */ ++ /* The CONTROL_PATH doesn't exist or is unusable. ++ * Probably the module isn't loaded, yet. Don't error ++ * out, just exit. */ + return 0; + } + +-- +2.31.0 + diff --git a/SOURCES/libvirt-virdevmapper-fix-stat-comparison-in-virDMSanitizepath.patch b/SOURCES/libvirt-virdevmapper-fix-stat-comparison-in-virDMSanitizepath.patch new file mode 100644 index 0000000..9bef718 --- /dev/null +++ b/SOURCES/libvirt-virdevmapper-fix-stat-comparison-in-virDMSanitizepath.patch @@ -0,0 +1,49 @@ +From 99e00695f1ab24e122e83087162a84b2bf2b81e2 Mon Sep 17 00:00:00 2001 +Message-Id: <99e00695f1ab24e122e83087162a84b2bf2b81e2@dist-git> +From: Pavel Hrdina +Date: Mon, 8 Mar 2021 12:57:35 +0100 +Subject: [PATCH] virdevmapper: fix stat comparison in virDMSanitizepath +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Introduced by commit <22494556542c676d1b9e7f1c1f2ea13ac17e1e3e> which +fixed a CVE. + +If the @path passed to virDMSanitizepath() is not a DM name or not a +path to DM name this function could return incorrect sanitized path as +it would always be the first device under /dev/mapper/. + +Signed-off-by: Pavel Hrdina +Reviewed-by: Peter Krempa +(cherry picked from commit f711fa9ad09f68ea7f0bcaf999fab9c06dc6a93e) + +Conflicts: +- src/util/virdevmapper.c: Context, the downstream has + VIR_STEAL_PTR() while the cherry picked commit uses + g_steal_pointer() already. + +Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1933557 +Signed-off-by: Michal Privoznik +Message-Id: +Reviewed-by: Ján Tomko +--- + src/util/virdevmapper.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/util/virdevmapper.c b/src/util/virdevmapper.c +index a04d9650a6..4994b4caef 100644 +--- a/src/util/virdevmapper.c ++++ b/src/util/virdevmapper.c +@@ -219,7 +219,7 @@ virDMSanitizepath(const char *path) + return NULL; + + if (stat(tmp, &sb[1]) == 0 && +- sb[0].st_rdev == sb[0].st_rdev) { ++ sb[0].st_rdev == sb[1].st_rdev) { + VIR_STEAL_PTR(ret, tmp); + break; + } +-- +2.31.0 + diff --git a/SOURCES/libvirt-virdevmapper.c-Join-two-WITH_DEVMAPPER-sections-together.patch b/SOURCES/libvirt-virdevmapper.c-Join-two-WITH_DEVMAPPER-sections-together.patch new file mode 100644 index 0000000..df558fa --- /dev/null +++ b/SOURCES/libvirt-virdevmapper.c-Join-two-WITH_DEVMAPPER-sections-together.patch @@ -0,0 +1,73 @@ +From b578178e72373c13f4e1e819793e28b9d39fc2f2 Mon Sep 17 00:00:00 2001 +Message-Id: +From: Michal Privoznik +Date: Mon, 8 Mar 2021 12:57:29 +0100 +Subject: [PATCH] virdevmapper.c: Join two WITH_DEVMAPPER sections together +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +There are two distinct WITH_DEVMAPPER sections in the file, for +different functions each. Rearrange the code to make some of +future commits smaller. + +Signed-off-by: Michal Privoznik +Reviewed-by: Daniel P. Berrangé +(cherry picked from commit ae5752aabc09f435675504246e30a0b9c4795d79) + +Conflicts: +-src/util/virdevmapper.c: The old code uses ATTRIBUTE_UNUSED, the +cherry picked commit used G_GNUC_UNUSED because of commit +679f8b3994457f7801b470b7c6a3379d844d0f79 which is not backported. + +Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1933557 +Signed-off-by: Michal Privoznik +Message-Id: <675e510b0dfca0f4b4f3dc3ed3952b763182bad3.1615203117.git.mprivozn@redhat.com> +Reviewed-by: Ján Tomko +--- + src/util/virdevmapper.c | 21 +++++++++------------ + 1 file changed, 9 insertions(+), 12 deletions(-) + +diff --git a/src/util/virdevmapper.c b/src/util/virdevmapper.c +index c69830ac70..6aee00112c 100644 +--- a/src/util/virdevmapper.c ++++ b/src/util/virdevmapper.c +@@ -201,19 +201,7 @@ virDevMapperGetTargets(const char *path, + return virDevMapperGetTargetsImpl(path, devPaths, ttl); + } + +-#else /* ! WITH_DEVMAPPER */ +- +-int +-virDevMapperGetTargets(const char *path ATTRIBUTE_UNUSED, +- char ***devPaths ATTRIBUTE_UNUSED) +-{ +- errno = ENOSYS; +- return -1; +-} +-#endif /* ! WITH_DEVMAPPER */ + +- +-#if WITH_DEVMAPPER + bool + virIsDevMapperDevice(const char *dev_name) + { +@@ -229,6 +217,15 @@ virIsDevMapperDevice(const char *dev_name) + + #else /* ! WITH_DEVMAPPER */ + ++int ++virDevMapperGetTargets(const char *path ATTRIBUTE_UNUSED, ++ char ***devPaths ATTRIBUTE_UNUSED) ++{ ++ errno = ENOSYS; ++ return -1; ++} ++ ++ + bool + virIsDevMapperDevice(const char *dev_name G_GNUC_UNUSED) + { +-- +2.31.0 + diff --git a/SPECS/libvirt.spec b/SPECS/libvirt.spec index 5b85981..4949ae0 100644 --- a/SPECS/libvirt.spec +++ b/SPECS/libvirt.spec @@ -253,7 +253,7 @@ Summary: Library providing a simple virtualization API Name: libvirt Version: 4.5.0 -Release: 36%{?dist}.3%{?extra_release} +Release: 36%{?dist}.5%{?extra_release} License: LGPLv2+ URL: https://libvirt.org/ @@ -652,6 +652,15 @@ Patch386: libvirt-rpc-add-support-for-filtering-acls-by-uint-params.patch Patch387: libvirt-rpc-require-write-acl-for-guest-agent-in-virDomainInterfaceAddresses.patch Patch388: libvirt-qemu-agent-set-ifname-to-NULL-after-freeing.patch Patch389: libvirt-conf-properly-clear-out-autogenerated-macvtap-names-when-formatting-parsing.patch +Patch390: libvirt-virdevmapper.c-Join-two-WITH_DEVMAPPER-sections-together.patch +Patch391: libvirt-virDevMapperGetTargetsImpl-Use-VIR_AUTOSTRINGLIST.patch +Patch392: libvirt-virdevmapper-Don-t-use-libdevmapper-to-obtain-dependencies.patch +Patch393: libvirt-virdevmapper-Don-t-cache-device-mapper-major.patch +Patch394: libvirt-virdevmapper-Handle-kernel-without-device-mapper-support.patch +Patch395: libvirt-virdevmapper-Ignore-all-errors-when-opening-dev-mapper-control.patch +Patch396: libvirt-virdevmapper-fix-stat-comparison-in-virDMSanitizepath.patch +Patch397: libvirt-virDevMapperGetTargetsImpl-Use-correct-length-when-copying-into-dm.name.patch +Patch398: libvirt-RHEL-virdevmapper-Don-t-leak-DIR-on-OOM-in-virDMSanitizepath.patch Requires: libvirt-daemon = %{version}-%{release} Requires: libvirt-daemon-config-network = %{version}-%{release} @@ -2554,6 +2563,19 @@ exit 0 %changelog +* Fri Mar 19 2021 Jiri Denemark - 4.5.0-36.el7_9.5 +- RHEL: virdevmapper: Don't leak DIR on OOM in virDMSanitizepath() (rhbz#1933557) + +* Thu Mar 18 2021 Jiri Denemark - 4.5.0-36.el7_9.4 +- virdevmapper.c: Join two WITH_DEVMAPPER sections together (rhbz#1933557) +- virDevMapperGetTargetsImpl: Use VIR_AUTOSTRINGLIST (rhbz#1933557) +- virdevmapper: Don't use libdevmapper to obtain dependencies (rhbz#1933557) +- virdevmapper: Don't cache device-mapper major (rhbz#1933557) +- virdevmapper: Handle kernel without device-mapper support (rhbz#1933557) +- virdevmapper: Ignore all errors when opening /dev/mapper/control (rhbz#1933557) +- virdevmapper: fix stat comparison in virDMSanitizepath (rhbz#1933557) +- virDevMapperGetTargetsImpl: Use correct length when copying into dm.name (rhbz#1933557) + * Tue Oct 20 2020 Jiri Denemark - 4.5.0-36.el7_9.3 - rpc: gendispatch: handle empty flags (CVE-2020-25637) - rpc: add support for filtering @acls by uint params (CVE-2020-25637)