Include upstream patch to fix double-free if appliance building fails (RHBZ#983218).

This commit is contained in:
Richard W.M. Jones 2013-07-11 21:57:42 +01:00
parent 0b709fcb5b
commit 397998a1b7
2 changed files with 150 additions and 2 deletions

View File

@ -0,0 +1,144 @@
From ae78381287771a781f939f26a414fc8cfdc05fd6 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Thu, 11 Jul 2013 19:09:05 +0100
Subject: [PATCH] launch: appliance: Fix a double-free if kernel linking fails
(RHBZ#983218).
This also simplifies the code which takes the building_lock around
guestfs___build_appliance.
Thanks Attila Fazekas for the detailed bug report.
---
src/appliance.c | 59 ++++++++++++++++++++++++++++++++++-----------------------
1 file changed, 35 insertions(+), 24 deletions(-)
diff --git a/src/appliance.c b/src/appliance.c
index a05e966..5481e79 100644
--- a/src/appliance.c
+++ b/src/appliance.c
@@ -45,6 +45,7 @@
static const char *kernel_name = "vmlinuz." host_cpu;
static const char *initrd_name = "initramfs." host_cpu ".img";
+static int build_appliance (guestfs_h *g, char **kernel, char **initrd, char **appliance);
static int find_path (guestfs_h *g, int (*pred) (guestfs_h *g, const char *pelem, void *data), void *data, char **pelem);
static int dir_contains_file (const char *dir, const char *file);
static int dir_contains_files (const char *dir, ...);
@@ -137,21 +138,44 @@ gl_lock_define_initialized (static, building_lock);
*/
int
guestfs___build_appliance (guestfs_h *g,
- char **kernel, char **initrd, char **appliance)
+ char **kernel_rtn,
+ char **initrd_rtn,
+ char **appliance_rtn)
+{
+ int r;
+ char *kernel, *initrd, *appliance;
+
+ gl_lock_lock (building_lock);
+ r = build_appliance (g, &kernel, &initrd, &appliance);
+ gl_lock_unlock (building_lock);
+
+ if (r == -1)
+ return -1;
+
+ /* Don't assign these until we know we're going to succeed, to avoid
+ * the caller double-freeing (RHBZ#983218).
+ */
+ *kernel_rtn = kernel;
+ *initrd_rtn = initrd;
+ *appliance_rtn = appliance;
+ return 0;
+}
+
+static int
+build_appliance (guestfs_h *g,
+ char **kernel,
+ char **initrd,
+ char **appliance)
{
int r;
uid_t uid = geteuid ();
CLEANUP_FREE char *supermin_path = NULL;
CLEANUP_FREE char *path = NULL;
- gl_lock_lock (building_lock);
-
/* Step (1). */
r = find_path (g, contains_supermin_appliance, NULL, &supermin_path);
- if (r == -1) {
- gl_lock_unlock (building_lock);
+ if (r == -1)
return -1;
- }
if (r == 1) {
/* Step (2): calculate checksum. */
@@ -161,25 +185,19 @@ guestfs___build_appliance (guestfs_h *g,
/* Step (3): cached appliance exists? */
r = check_for_cached_appliance (g, supermin_path, checksum, uid,
kernel, initrd, appliance);
- if (r != 0) {
- gl_lock_unlock (building_lock);
+ if (r != 0)
return r == 1 ? 0 : -1;
- }
/* Step (4): build supermin appliance. */
- r = build_supermin_appliance (g, supermin_path, checksum, uid,
- kernel, initrd, appliance);
- gl_lock_unlock (building_lock);
- return r;
+ return build_supermin_appliance (g, supermin_path, checksum, uid,
+ kernel, initrd, appliance);
}
}
/* Step (5). */
r = find_path (g, contains_fixed_appliance, NULL, &path);
- if (r == -1) {
- gl_lock_unlock (building_lock);
+ if (r == -1)
return -1;
- }
if (r == 1) {
size_t len = strlen (path);
@@ -189,17 +207,13 @@ guestfs___build_appliance (guestfs_h *g,
sprintf (*kernel, "%s/kernel", path);
sprintf (*initrd, "%s/initrd", path);
sprintf (*appliance, "%s/root", path);
-
- gl_lock_unlock (building_lock);
return 0;
}
/* Step (6). */
r = find_path (g, contains_old_style_appliance, NULL, &path);
- if (r == -1) {
- gl_lock_unlock (building_lock);
+ if (r == -1)
return -1;
- }
if (r == 1) {
size_t len = strlen (path);
@@ -208,14 +222,11 @@ guestfs___build_appliance (guestfs_h *g,
sprintf (*kernel, "%s/%s", path, kernel_name);
sprintf (*initrd, "%s/%s", path, initrd_name);
*appliance = NULL;
-
- gl_lock_unlock (building_lock);
return 0;
}
error (g, _("cannot find any suitable libguestfs supermin, fixed or old-style appliance on LIBGUESTFS_PATH (search path: %s)"),
g->path);
- gl_lock_unlock (building_lock);
return -1;
}
--
1.8.3.1

View File

@ -12,7 +12,7 @@ Summary: Access and modify virtual machine disk images
Name: libguestfs
Epoch: 1
Version: 1.23.8
Release: 4%{?dist}
Release: 5%{?dist}
License: LGPLv2+
# Source and patches.
@ -23,6 +23,7 @@ Patch1: 0001-golang-Fix-it-so-it-builds-if-libguestfs-is-not-inst.patch
Patch2: 0001-.-run-Add-a-better-comment-describing-test-mode.patch
Patch3: 0002-.-run-Timeout-tests-after-1-hour.patch
Patch4: 0001-.-run-Increase-default-timeout-from-1h-4h.patch
Patch5: 0001-launch-appliance-Fix-a-double-free-if-kernel-linking.patch
# Basic build requirements:
BuildRequires: perl(Pod::Simple)
@ -543,6 +544,7 @@ for %{name}.
%patch2 -p1
%patch3 -p1
%patch4 -p1
%patch5 -p1
if [ "$(getenforce | tr '[A-Z]' '[a-z]')" != "disabled" ]; then
# For sVirt to work, the local temporary directory we use in the
@ -916,8 +918,10 @@ mkdir -p $RPM_BUILD_ROOT%{_localstatedir}/run/libguestfs
%changelog
* Thu Jul 11 2013 Richard W.M. Jones <rjones@redhat.com> - 1:1.23.8-4
* Thu Jul 11 2013 Richard W.M. Jones <rjones@redhat.com> - 1:1.23.8-5
- Add patches to ./run so we capture errors when i686 tests time out.
- Include upstream patch to fix double-free if appliance
building fails (RHBZ#983218).
* Tue Jul 9 2013 Richard W.M. Jones <rjones@redhat.com> - 1:1.23.8-2
- New upstream version 1.23.8.