December 2009 - Devel - libvirt List Archives

[libvirt] [PATCH] esx_vmx.c: don't dereference a NULL virtualDev
by Jim Meyering 16 Dec '09

16 Dec '09

The offending code starts here: int esxVMX_ParseSCSIController(virConnectPtr conn, virConfPtr conf, int controller, int *present, char **virtualDev) { char present_name[32]; char virtualDev_name[32]; if (virtualDev == NULL || *virtualDev != NULL) { ESX_ERROR(conn, VIR_ERR_INTERNAL_ERROR, "Invalid argument"); goto failure; } If the virtualDev parameter is NULL, then we'd issue the diagnostic and take the "goto", and (below), dereference NULL. >From 79283ba1d667534175d4c48079e6b500feba6480 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 15 Dec 2009 16:07:10 +0100 Subject: [PATCH] esx_vmx.c: don't dereference NULL for a NULL virtualDev * src/esx/esx_vmx.c (esxVMX_ParseSCSIController): Don't deref "virtualDev" when it is NULL. --- src/esx/esx_vmx.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/esx/esx_vmx.c b/src/esx/esx_vmx.c index f5b4544..404617e 100644 --- a/src/esx/esx_vmx.c +++ b/src/esx/esx_vmx.c @@ -1204,7 +1204,8 @@ esxVMX_ParseSCSIController(virConnectPtr conn, virConfPtr conf, int controller, return 0; failure: - VIR_FREE(*virtualDev); + if (virtualDev) + VIR_FREE(*virtualDev); return -1; } -- 1.6.6.rc2.275.g51e2d

4 5

[libvirt] [PATCH] Fix memory leak in qemudBuildCommandLine
by Matthias Bolte 16 Dec '09

16 Dec '09

--- src/qemu/qemu_conf.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 86172c6..2ee2042 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1893,7 +1893,7 @@ int qemudBuildCommandLine(virConnectPtr conn, } ADD_ARG_LIT("-monitor"); - ADD_ARG_LIT(virBufferContentAndReset(&buf)); + ADD_ARG(virBufferContentAndReset(&buf)); } if (def->localtime) -- 1.6.0.4

2 2

[libvirt] the <filesystem> tag (WAS: simple LXC/libvirt busybox container (Unable to get cgroup))
by Tony Risinger 16 Dec '09

16 Dec '09

wow! i was starting to think i would never get past this problem! [root@PHS-001 ~]# echo $VIRSH_DEFAULT_CONNECT_URI lxc:/// [root@PHS-001 ~]# virsh create /vps/def/exec/sys/arch-nano.xml Domain arch-nano created from /vps/def/exec/sys/arch-nano.xml [root@PHS-001 ~]# virsh console arch-nano Connected to domain arch-nano Escape character is ^] # [root@PHS-001 ~]# grep cgroup /proc/mounts none /cgroup cgroup rw,relatime,devices,memory,cpuacct,cpu 0 0 everything seems to be working fine now after disabling "ns" from cgroup mount; thank you very much Ryota and Daniel for you time and assistance. i must have sunk 40+ hours into this, but i learned a tremendous amount, so nothing is lost. :-) this is somewhat of a digression, and i can submit a new thread if need be, but my last remaining question is about the <filesystem> tag/root mounting. i have not found any mention of the <filesystem> tag in the docs or how to work with it... i want to have libvirt mount a specified device (a btrfs subvolume) and use it for the root filesystem of the created LXC container. is there a way to do this? perhaps <source dev="/dev/sdb" options="subvol....."/>, or do i need to use the storage XML? i didnt see a way to use a storage/device definition as the rootfs. im eager to get working with python bindings, just trying to see it all work with XML first. thanks++ On Thu, Dec 10, 2009 at 6:45 AM, Ryota Ozaki <ozaki.ryota(a)gmail.com> wrote: > On Thu, Dec 10, 2009 at 9:36 PM, Daniel P. Berrange <berrange(a)redhat.com> wrote: >> On Thu, Dec 10, 2009 at 09:26:39PM +0900, Ryota Ozaki wrote: >>> On Thu, Dec 10, 2009 at 9:03 PM, Daniel P. Berrange <berrange(a)redhat.com> wrote: >>> > On Thu, Dec 10, 2009 at 02:22:37AM -0600, Tony Risinger wrote: >>> >> i'm trying to get even the simplest busybox container with libvirt+LXC >>> >> with very limited success. I feel l am missing something supremely >>> >> simple for me to be hung on this for weeks. >>> >> >>> >> i dont see anything interesting in domain log, but getting this error >>> >> from "LIBVIRT_DEBUG=1 libvirtd": >>> >> >>> >> 05:27:56.113: error : lxcDomainGetInfo:462 : internal error Unable to >>> >> get cgroup for arch-nano >>> >> 05:27:56.113: debug : virDomainFree:2004 : domain=0x81d8e68 >>> >> 05:27:56.113: debug : virUnrefDomain:422 : unref domain 0x81d8e68 arch-nano 1 >>> >> 05:27:56.113: debug : virReleaseDomain:376 : release domain 0x81d8e68 arch-nano >>> >> 05:27:56.113: debug : virReleaseDomain:392 : unref connection 0x81dc0f0 2 >>> >> 05:27:56.113: debug : remoteSerializeError:141 : prog=536903814 ver=1 >>> >> proc=16 type=1 serial=4, msg=internal error Unable to get cgroup for >>> >> arch-nano >>> >> >>> >> i've been using this root filesystem layout: >>> >> >>> >> [root@PHS-001 arch-nano]# tree >>> >> . >>> >> |-- bin >>> >> | |-- cat -> ../sbin/busybox >>> >> | |-- chdir -> ../sbin/busybox >>> >> | |-- chmod -> ../sbin/busybox >>> >> | |-- ls -> ../sbin/busybox >>> >> | |-- rm -> ../sbin/busybox >>> >> | |-- sh -> ../sbin/busybox >>> >> | `-- vi -> ../sbin/busybox >>> >> |-- dev >>> >> | `-- pts >>> >> |-- etc >>> >> |-- proc >>> >> |-- sbin >>> >> | |-- busybox >>> >> | `-- init -> busybox >>> >> `-- sys >>> >> >>> >> all folders besides /bin and /sbin were created by libvirt. i tried >>> >> using the /sbin/init script previously suggested: >>> >> >>> >> #!/sbin/busybox >>> >> sh >>> > >>> > >>> > Sorry, my suggestion was wrong. I forgot that if you have #!/sbin/busybox >>> > it will attempt to execute the command matching the name of the script. >>> > So it will in fact try to run 'init', rather than 'sh'. >>> > >>> > Just make the libvirt XML point directly to /bin/sh instead and it >>> > should work. I even tested it this time :-) >>> >>> Hem, I still have a problem with ns subsystem enabled. Yes, I can launch >>> a container however the cgroup hierarchy is wrong from libvirtd expecting >>> like: >>> >>> /: libvirtd --daemon >>> /5345: /usr/libexec/libvirt_lxc --name >>> >>> Daniel, could you confirm how about your cgroup hierarchy? >> >> What you do mean by 'ns' subsystem ? > > 'ns' is one of functions of cgroups like such as devices, memory, > cpu, etc. and it is enabled if you mount cgroup without any options > that Tony is doing. > >> >> >> # grep cgroup /proc/mounts >> cgroup /dev/cgroups/cpu cgroup rw,relatime,cpuacct,cpu 0 0 >> cgroup /dev/cgroups/memory cgroup rw,relatime,memory 0 0 >> cgroup /dev/cgroups/devices cgroup rw,relatime,devices 0 0 > > Oh, you don't enable 'ns', so yes, things go fine in your environment. > > >> >> # cat /proc/`pgrep libvirtd`/cgroup >> 32:devices:/sysdefault >> 16:memory:/sysdefault >> 12:cpuacct,cpu:/sysdefault >> >> # cat /proc/`pgrep libvirt_lxc`/cgroup >> 32:devices:/sysdefault/libvirt/lxc/vm1 >> 16:memory:/sysdefault/libvirt/lxc/vm1 >> 12:cpuacct,cpu:/sysdefault/libvirt/lxc/vm1 >> >> >> And the process inside the contanier is PID 12309 >> >> # cat /proc/12309/cgroup >> 32:devices:/sysdefault/libvirt/lxc/vm1 >> 16:memory:/sysdefault/libvirt/lxc/vm1 >> 12:cpuacct,cpu:/sysdefault/libvirt/lxc/vm1 >> >> >> Which all appears to be correct to me >> >> This is on a Fedora 12 host 2.6.31.6-145.fc12.i686.PAE with >> >> CONFIG_UTS_NS=y >> CONFIG_IPC_NS=y >> CONFIG_USER_NS=y >> CONFIG_PID_NS=y >> CONFIG_NET_NS=y >> CONFIG_CGROUP_SCHED=y >> CONFIG_CGROUPS=y >> # CONFIG_CGROUP_DEBUG is not set >> CONFIG_CGROUP_NS=y > > This is the function I'm mentioning. > > ozaki-r > >> CONFIG_CGROUP_FREEZER=y >> CONFIG_CGROUP_DEVICE=y >> CONFIG_CGROUP_CPUACCT=y >> CONFIG_CGROUP_MEM_RES_CTLR=y >> CONFIG_CGROUP_MEM_RES_CTLR_SWAP=y >> CONFIG_NET_CLS_CGROUP=y >> >> >> Regards, >> Daniel >> -- >> |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| >> |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| >> |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| >> |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| >> >

2 2

[libvirt] [PATCH] esx_vi.c: avoid NULL deref for invalid inputs
by Jim Meyering 16 Dec '09

16 Dec '09

The first statement in this function tests for freeFunc == NULL, and if that's true, it does "goto failure" where it proceeds to call through that NULL function pointer: if (list == NULL || *list != NULL || castFromAnyTypeFunc == NULL || freeFunc == NULL) { ESX_VI_ERROR(conn, VIR_ERR_INTERNAL_ERROR, "Invalid argument"); return -1; } ... failure: freeFunc(&item); freeFunc(list); >From 97c5e9c87eb99d881ee84c4f09bb943f3410d3b1 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 15 Dec 2009 19:22:31 +0100 Subject: [PATCH] esx_vi.c: avoid NULL deref for invalid inputs * src/esx/esx_vi.c (esxVI_List_CastFromAnyType): For invalid inputs, fail right away. Do not "goto failure" where a NULL input pointer would be dereferenced. --- src/esx/esx_vi.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 5725b2f..1138e8c 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -935,7 +935,7 @@ esxVI_List_CastFromAnyType(virConnectPtr conn, esxVI_AnyType *anyType, if (list == NULL || *list != NULL || castFromAnyTypeFunc == NULL || freeFunc == NULL) { ESX_VI_ERROR(conn, VIR_ERR_INTERNAL_ERROR, "Invalid argument"); - goto failure; + return -1; } if (anyType == NULL) { @@ -946,7 +946,7 @@ esxVI_List_CastFromAnyType(virConnectPtr conn, esxVI_AnyType *anyType, ESX_VI_ERROR(conn, VIR_ERR_INTERNAL_ERROR, "Expecting type to begin with 'ArrayOf' but found '%s'", anyType->other); - goto failure; + return -1; } for (childNode = anyType->_node->xmlChildrenNode; childNode != NULL; -- 1.6.6.rc2.275.g51e2d

2 1

[libvirt] [PATCH] libvirt.c: don't let a NULL "cpumaps" argument provoke a NULL-deref
by Jim Meyering 15 Dec '09

15 Dec '09

>From 94923b161a9d066146271bb533b78ab7877e4501 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 14 Dec 2009 17:17:53 +0100 Subject: [PATCH] libvirt.c: don't let a NULL "cpumaps" argument provoke a NULL-deref * src/libvirt.c (virDomainGetVcpus): Update spec to say that maplen is ignored when "cpumaps" is NULL. Set maplen to 0 in that case. --- src/libvirt.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 008e322..4325aa4 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -4753,6 +4753,7 @@ error: * virDomainPinVcpu() API. * @maplen: number of bytes in one cpumap, from 1 up to size of CPU map in * underlying virtualization system (Xen...). + * Ignored when cpumaps is NULL. * * Extract information about virtual CPUs of domain, store it in info array * and also in cpumaps if this pointer isn't NULL. @@ -4776,6 +4777,12 @@ virDomainGetVcpus(virDomainPtr domain, virVcpuInfoPtr info, int maxinfo, virLibDomainError(domain, VIR_ERR_INVALID_ARG, __FUNCTION__); goto error; } + + /* Ensure that domainGetVcpus (aka remoteDomainGetVcpus) does not + try to memcpy anything into a NULL pointer. */ + if (cpumaps == NULL) + maplen = 0; + if (cpumaps != NULL && maplen < 1) { virLibDomainError(domain, VIR_ERR_INVALID_ARG, __FUNCTION__); goto error; -- 1.6.6.rc2.275.g51e2d

3 4

[libvirt] [PATCH] xm_internal.c: remove misleading dead code
by Jim Meyering 15 Dec '09

15 Dec '09

This code triggered a warning about possible NULL-dereference. Actually it's more about being self-contradictory, since the NULL-dereference is not possible. >From 796e3a3254cb08fc20bce790997b5787dab51902 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 14 Dec 2009 21:37:54 +0100 Subject: [PATCH] xm_internal.c: remove misleading dead code * src/xen/xm_internal.c (xenXMConfigGetULong): Remove useless and misleading test (always false) for val->str == NULL before code that always dereferences val->str. "val" comes from virConfGetValue, and at that point, val->str is guaranteed to be non-NULL. (xenXMConfigGetBool): Likewise. --- src/xen/xm_internal.c | 6 ------ 1 files changed, 0 insertions(+), 6 deletions(-) diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 3dc655b..944d5d5 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -152,9 +152,6 @@ static int xenXMConfigGetBool(virConnectPtr conn, if (val->type == VIR_CONF_LONG) { *value = val->l ? 1 : 0; } else if (val->type == VIR_CONF_STRING) { - if (!val->str) { - *value = def; - } *value = STREQ(val->str, "1") ? 1 : 0; } else { xenXMError(conn, VIR_ERR_INTERNAL_ERROR, @@ -183,9 +180,6 @@ static int xenXMConfigGetULong(virConnectPtr conn, *value = val->l; } else if (val->type == VIR_CONF_STRING) { char *ret; - if (!val->str) { - *value = def; - } *value = strtol(val->str, &ret, 10); if (ret == val->str) { xenXMError(conn, VIR_ERR_INTERNAL_ERROR, -- 1.6.6.rc2.275.g51e2d

3 4

[libvirt] [PATCH] avoid calling exit with a constant; use EXIT_* instead
by Jim Meyering 15 Dec '09

15 Dec '09

Without this, and with updated gnulib, "make syntax-check" would fail, complaining about exit(1), exit(0), exit(77). It's no big deal, but slightly better for readability to use EXIT_SUCCESS, EXIT_FAILURE, and the new, made-up/defined EXIT_AM_SKIP. The only semantic change was to convert the "exit(2)" and "exit(3)" in tests/conftest.c to "exit(EXIT_FAILURE);". That seems ok, since the sole invoker of that test program cares only about success/failure, and not about the precise exit code. >From 8dba638f597b50f295f3f9b5628002f39b16d776 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 15 Dec 2009 09:43:29 +0100 Subject: [PATCH] avoid calling exit with a constant; use EXIT_* instead This appeases a new gnulib-provided "syntax-check". * daemon/libvirtd.c (main): Use EXIT_FAILURE, not 1. * proxy/libvirt_proxy.c (main): Likewise, and EXIT_SUCCESS, not 0. * tests/conftest.c (main): Likewise. * tests/reconnect.c (main): Likewise. * tests/testutils.h (EXIT_AM_SKIP): Define. * tests/nodeinfotest.c (mymain): Use EXIT_AM_SKIP, not 77. * tests/qemuargv2xmltest.c: Likewise. * tests/qemuxml2xmltest.c: Likewise. * tests/virshtest.c (mymain): Likewise. --- daemon/libvirtd.c | 2 +- proxy/libvirt_proxy.c | 10 +++++----- tests/conftest.c | 10 +++++----- tests/nodeinfotest.c | 2 +- tests/qemuargv2xmltest.c | 2 +- tests/qemuxml2xmltest.c | 2 +- tests/reconnect.c | 10 +++++----- tests/testutils.h | 4 +++- tests/virshtest.c | 2 +- 9 files changed, 23 insertions(+), 21 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index de6fc27..281a46a 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -3036,7 +3036,7 @@ int main(int argc, char **argv) { default: fprintf (stderr, "libvirtd: internal error: unknown flag: %c\n", c); - exit (1); + exit (EXIT_FAILURE); } } diff --git a/proxy/libvirt_proxy.c b/proxy/libvirt_proxy.c index e5298fd..375966c 100644 --- a/proxy/libvirt_proxy.c +++ b/proxy/libvirt_proxy.c @@ -816,14 +816,14 @@ int main(int argc, char **argv) { persist = 1; } else { usage(argv[0]); - exit(1); + exit(EXIT_FAILURE); } } if (geteuid() != 0) { fprintf(stderr, "%s must be run as root or suid\n", argv[0]); - /* exit(1); */ + /* exit(EXIT_FAILURE); */ } /* @@ -838,19 +838,19 @@ int main(int argc, char **argv) { * failure. */ if (proxyListenUnixSocket(PROXY_SOCKET_PATH) < 0) - exit(0); + exit(EXIT_SUCCESS); if (proxyInitXen() == 0) proxyMainLoop(); sleep(1); proxyCloseUnixSocket(); - exit(0); + exit(EXIT_SUCCESS); } #else /* WITHOUT_XEN */ int main(void) { fprintf(stderr, "libvirt was compiled without Xen support\n"); - exit(1); + exit(EXIT_FAILURE); } #endif /* WITH_XEN */ diff --git a/tests/conftest.c b/tests/conftest.c index d265de2..a7977bb 100644 --- a/tests/conftest.c +++ b/tests/conftest.c @@ -15,23 +15,23 @@ int main(int argc, char **argv) { if (argc != 2) { fprintf(stderr, "Usage: %s conf_file\n", argv[0]); - exit(1); + exit(EXIT_FAILURE); } conf = virConfReadFile(argv[1], 0); if (conf == NULL) { fprintf(stderr, "Failed to process %s\n", argv[1]); - exit(2); + exit(EXIT_FAILURE); } ret = virConfWriteMem(&buffer[0], &len, conf); if (ret < 0) { fprintf(stderr, "Failed to serialize %s back\n", argv[1]); - exit(3); + exit(EXIT_FAILURE); } virConfFree(conf); if (fwrite(buffer, 1, len, stdout) != len) { fprintf(stderr, "Write failed: %s\n", strerror (errno)); - exit(1); + exit(EXIT_FAILURE); } - exit(0); + exit(EXIT_SUCCESS); } diff --git a/tests/nodeinfotest.c b/tests/nodeinfotest.c index 608a056..4cb248a 100644 --- a/tests/nodeinfotest.c +++ b/tests/nodeinfotest.c @@ -15,7 +15,7 @@ static int mymain(int argc ATTRIBUTE_UNUSED, char **argv ATTRIBUTE_UNUSED) { - exit (77); /* means 'test skipped' for automake */ + exit (EXIT_AM_SKIP); } #else diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 5602df0..7f62bac 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -232,6 +232,6 @@ VIRT_TEST_MAIN(mymain) #else -int main (void) { return (77); /* means 'test skipped' for automake */ } +int main (void) { return (EXIT_AM_SKIP); } #endif /* WITH_QEMU */ diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 793900c..e5900a2 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -146,6 +146,6 @@ VIRT_TEST_MAIN(mymain) #else -int main (void) { exit (77); /* means 'test skipped' to automake */ } +int main (void) { exit (EXIT_AM_SKIP); } #endif /* WITH_QEMU */ diff --git a/tests/reconnect.c b/tests/reconnect.c index 33af2cc..63877fc 100644 --- a/tests/reconnect.c +++ b/tests/reconnect.c @@ -24,12 +24,12 @@ int main(void) { } if (conn == NULL) { fprintf(stderr, "First virConnectOpen() failed\n"); - exit(1); + exit(EXIT_FAILURE); } dom = virDomainLookupByID(conn, id); if (dom == NULL) { fprintf(stderr, "First lookup for domain %d failed\n", id); - exit(1); + exit(EXIT_FAILURE); } virDomainFree(dom); virConnectClose(conn); @@ -39,16 +39,16 @@ int main(void) { conn = virConnectOpen(NULL); if (conn == NULL) { fprintf(stderr, "Second virConnectOpen() failed\n"); - exit(1); + exit(EXIT_FAILURE); } dom = virDomainLookupByID(conn, id); if (dom == NULL) { fprintf(stderr, "Second lookup for domain %d failed\n", id); - exit(1); + exit(EXIT_FAILURE); } virDomainFree(dom); virConnectClose(conn); printf("OK\n"); - exit(0); + exit(EXIT_SUCCESS); } diff --git a/tests/testutils.h b/tests/testutils.h index aef1179..e5d5750 100644 --- a/tests/testutils.h +++ b/tests/testutils.h @@ -1,7 +1,7 @@ /* * utils.c: test utils * - * Copyright (C) 2005, 2008 Red Hat, Inc. + * Copyright (C) 2005, 2008-2009 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -13,6 +13,8 @@ #include <stdio.h> +#define EXIT_AM_SKIP 77 /* tell Automake we're skipping a test */ + double virtTestCountAverage(double *items, int nitems); diff --git a/tests/virshtest.c b/tests/virshtest.c index ef760e2..ad3e2fc 100644 --- a/tests/virshtest.c +++ b/tests/virshtest.c @@ -231,7 +231,7 @@ mymain(int argc, char **argv) abs_srcdir = getcwd(cwd, sizeof(cwd)); #ifdef WIN32 - exit (77); /* means 'test skipped' for automake */ + exit (EXIT_AM_SKIP); #endif snprintf(buffer, PATH_MAX-1, "test://%s/../examples/xml/test/testnode.xml", abs_srcdir); -- 1.6.6.rc2.275.g51e2d

3 2

[libvirt] [PATCH] avoid malfunction when virFileResolveLink is applied to non-POSIX FS
by Jim Meyering 15 Dec '09

15 Dec '09

Without this patch, virFileResolveLink could misbehave when applied to a file on a POSIX-nonconforming file system. The fix is to use gnulib's areadlink module, but it's GPL'd. I'm ready to relax it to LGPLv2+, and have asked the other author for his rubber stamp. Once that's fixed, this change will depend on having an up-to-date gnulib submodule. That will be the subject of my next message. >From a3133b2e8b1453578b30e4b9c83c7473feb7c65b Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 15 Dec 2009 08:27:53 +0100 Subject: [PATCH] avoid malfunction when virFileResolveLink is applied to non-POSIX FS The virFileResolveLink utility function relied on the POSIX guarantee that stat.st_size of a symlink is the length of the value. However, on some types of file systems, it is invalid, so do not rely on it. Use gnulib's areadlink module instead. * bootstrap (modules): Add areadlink. * src/util/util.c: Include "areadlink.h". Let areadlink perform the readlink and malloc. * configure.in (AC_CHECK_FUNCS): Remove readlink. No need, since it's presence is guaranteed by gnulib. --- bootstrap | 1 + configure.in | 2 +- src/util/util.c | 27 +++------------------------ 3 files changed, 5 insertions(+), 25 deletions(-) diff --git a/bootstrap b/bootstrap index 667af4f..c07d851 100755 --- a/bootstrap +++ b/bootstrap @@ -65,6 +65,7 @@ gnulib_tool=$GNULIB_SRCDIR/gnulib-tool <$gnulib_tool || exit modules=' +areadlink base64 c-ctype close diff --git a/configure.in b/configure.in index 6135932..6ed2efd 100644 --- a/configure.in +++ b/configure.in @@ -83,7 +83,7 @@ dnl Use --disable-largefile if you don't want this. AC_SYS_LARGEFILE dnl Availability of various common functions (non-fatal if missing). -AC_CHECK_FUNCS([cfmakeraw regexec uname sched_getaffinity getuid getgid posix_fallocate mmap readlink]) +AC_CHECK_FUNCS([cfmakeraw regexec uname sched_getaffinity getuid getgid posix_fallocate mmap]) dnl Availability of various not common threadsafe functions AC_CHECK_FUNCS([strerror_r strtok_r getmntent_r getgrnam_r getpwuid_r]) diff --git a/src/util/util.c b/src/util/util.c index 694838a..44a4b2f 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -64,6 +64,7 @@ #include <mntent.h> #endif +#include "areadlink.h" #include "virterror_internal.h" #include "logging.h" #include "event.h" @@ -1059,10 +1060,7 @@ int virFileLinkPointsTo(const char *checkLink, int virFileResolveLink(const char *linkpath, char **resultpath) { -#ifdef HAVE_READLINK struct stat st; - char *buf; - int n; *resultpath = NULL; @@ -1075,28 +1073,9 @@ int virFileResolveLink(const char *linkpath, return 0; } - /* Posix says that 'st_size' field from - * result of an lstat() call is filled with - * number of bytes in the destination - * filename. - */ - if (VIR_ALLOC_N(buf, st.st_size + 1) < 0) - return -ENOMEM; - - if ((n = readlink(linkpath, buf, st.st_size)) < 0) { - VIR_FREE(buf); - return -errno; - } - - buf[n] = '\0'; + *resultpath = areadlink (linkpath); - *resultpath = buf; - return 0; -#else - if (!(*resultpath = strdup(linkpath))) - return -ENOMEM; - return 0; -#endif + return *resultpath == NULL ? -1 : 0; } /* -- 1.6.6.rc2.275.g51e2d

3 3

[libvirt] [PATCH] qemu migration: avoid NULL-deref given an invalid input
by Jim Meyering 15 Dec '09

15 Dec '09

The fix here is to add the missing "goto". Nearly every other use of qemudReportError is followed closely by a goto or a return statement. >From 898033ec5bb0399f7cc6a22ffd9f351552ced85b Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 14 Dec 2009 17:02:56 +0100 Subject: [PATCH] qemu migration: avoid NULL-deref given an invalid input * src/qemu/qemu_driver.c (doNonTunnelMigrate): Don't let a NULL "uri_out" provoke a NULL-dereference in doNativeMigrate: supply omitted goto-after-qemudReportError. --- src/qemu/qemu_driver.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c782f0d..9ef6c35 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7430,6 +7430,7 @@ static int doNonTunnelMigrate(virDomainPtr dom, if (uri_out == NULL) { qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s", _("domainMigratePrepare2 did not set uri")); + goto cleanup; } if (doNativeMigrate(dom, driver, vm, uri_out, flags, dname, resource) < 0) -- 1.6.6.rc2.275.g51e2d

3 2

[libvirt] [PATCH] qemu_driver.c: don't unlink(NULL) on OOM error path
by Jim Meyering 15 Dec '09

15 Dec '09

Not terribly likely, but not impossible, either: >From dc8fe76ed209d04b1e962b94b20da17a077e102f Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 14 Dec 2009 16:41:11 +0100 Subject: [PATCH] qemu_driver.c: don't unlink(NULL) on OOM error path * src/qemu/qemu_driver.c (qemudDomainMigratePrepareTunnel): Upon an out of memory error, we would end up with unixfile==NULL and attempt to unlink(NULL). Skip the unlink when it's NULL. --- src/qemu/qemu_driver.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 56bcec5..04b2511 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6903,7 +6903,8 @@ endjob: cleanup: virDomainDefFree(def); - unlink(unixfile); + if (unixfile) + unlink(unixfile); VIR_FREE(unixfile); if (vm) virDomainObjUnlock(vm); -- 1.6.6.rc2.275.g51e2d

3 3