[libvirt] [PATCH] storage_backend_fs: avoid NULL dereference on opendir failure
I've just begun using clang's static analyzer, http://clang-analyzer.llvm.org/ It has uncovered a few problems in libvirt. Here are the first few fixes. I'll send more details later today.
From b6bb9d82effa56733fbee9013e66fed384d9ff63 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Wed, 2 Sep 2009 09:42:32 +0200 Subject: [PATCH 1/4] storage_backend_fs: avoid NULL dereference on opendir failure
* src/storage_backend_fs.c (virStorageBackendFileSystemRefresh): Don't call closedir on a NULL pointer. --- src/storage_backend_fs.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c index 65b656d..8241504 100644 --- a/src/storage_backend_fs.c +++ b/src/storage_backend_fs.c @@ -983,7 +983,8 @@ no_memory: /* fallthrough */ cleanup: - closedir(dir); + if (dir) + closedir(dir); virStorageVolDefFree(vol); virStoragePoolObjClearVols(pool); return -1; -- 1.6.4.2.395.ge3d52
From eaae148291680a72d19aa9d5320f90b98f123746 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Wed, 2 Sep 2009 09:58:28 +0200 Subject: [PATCH 2/4] storage_conf.c: avoid overflow upon use of "z" or "Z" (zebi) suffix
* src/storage_conf.c (virStorageSize): Don't try to compute 1024^7, since it's too large for a 64-bit type. --- src/storage_conf.c | 6 ------ 1 files changed, 0 insertions(+), 6 deletions(-) diff --git a/src/storage_conf.c b/src/storage_conf.c index c446069..110f0ad 100644 --- a/src/storage_conf.c +++ b/src/storage_conf.c @@ -919,12 +919,6 @@ virStorageSize(virConnectPtr conn, 1024ull; break; - case 'z': - case 'Z': - mult = 1024ull * 1024ull * 1024ull * 1024ull * 1024ull * - 1024ull * 1024ull; - break; - default: virStorageReportError(conn, VIR_ERR_XML_ERROR, _("unknown size units '%s'"), unit); -- 1.6.4.2.395.ge3d52
From 7f453c68bc709d542e4c40a388c92c7969ad0a3a Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Wed, 2 Sep 2009 09:58:50 +0200 Subject: [PATCH 3/4] lxc: avoid NULL dereference when we find no mount point
* src/lxc_container.c (lxcContainerUnmountOldFS): Don't pass a NULL pointer to qsort. --- src/lxc_container.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/lxc_container.c b/src/lxc_container.c index 950dd50..2073864 100644 --- a/src/lxc_container.c +++ b/src/lxc_container.c @@ -546,8 +546,9 @@ static int lxcContainerUnmountOldFS(void) } endmntent(procmnt); - qsort(mounts, nmounts, sizeof(mounts[0]), - lxcContainerChildMountSort); + if (mounts) + qsort(mounts, nmounts, sizeof(mounts[0]), + lxcContainerChildMountSort); for (i = 0 ; i < nmounts ; i++) { VIR_DEBUG("Umount %s", mounts[i]); -- 1.6.4.2.395.ge3d52
From 4e97befca175af427ed3b75f59e67cd620ee3ce2 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Wed, 2 Sep 2009 10:02:49 +0200 Subject: [PATCH 4/4] lxc: don't unlink(NULL) in main
* src/lxc_controller.c (main): Unlink sockpath only if it's non-NULL. --- src/lxc_controller.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/lxc_controller.c b/src/lxc_controller.c index 8d11238..914c10a 100644 --- a/src/lxc_controller.c +++ b/src/lxc_controller.c @@ -803,7 +803,8 @@ cleanup: if (def) virFileDeletePid(LXC_STATE_DIR, def->name); lxcControllerCleanupInterfaces(nveths, veths); - unlink(sockpath); + if (sockpath): + unlink(sockpath); VIR_FREE(sockpath); return rc; -- 1.6.4.2.395.ge3d52
On Wed, Sep 02, 2009 at 10:05:03AM +0200, Jim Meyering wrote:
I've just begun using clang's static analyzer,
http://clang-analyzer.llvm.org/
It has uncovered a few problems in libvirt. Here are the first few fixes. I'll send more details later today.
All patches look fine, it's just too bad we have to drop the 'z' storage size suffix, but it's not realistic on current platforms. ACK Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Wed, Sep 02, 2009 at 10:05:03AM +0200, Jim Meyering wrote:
From 7f453c68bc709d542e4c40a388c92c7969ad0a3a Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Wed, 2 Sep 2009 09:58:50 +0200 Subject: [PATCH 3/4] lxc: avoid NULL dereference when we find no mount point
* src/lxc_container.c (lxcContainerUnmountOldFS): Don't pass a NULL pointer to qsort. --- src/lxc_container.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/lxc_container.c b/src/lxc_container.c index 950dd50..2073864 100644 --- a/src/lxc_container.c +++ b/src/lxc_container.c @@ -546,8 +546,9 @@ static int lxcContainerUnmountOldFS(void) } endmntent(procmnt);
- qsort(mounts, nmounts, sizeof(mounts[0]), - lxcContainerChildMountSort); + if (mounts) + qsort(mounts, nmounts, sizeof(mounts[0]), + lxcContainerChildMountSort);
for (i = 0 ; i < nmounts ; i++) { VIR_DEBUG("Umount %s", mounts[i]);
This would is impossible to hit, since you must at least have a /proc filesystem if we've got this far, but doesn't hurt to check anyway :-) ACK 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 :|
Daniel P. Berrange wrote:
On Wed, Sep 02, 2009 at 10:05:03AM +0200, Jim Meyering wrote:
From 7f453c68bc709d542e4c40a388c92c7969ad0a3a Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Wed, 2 Sep 2009 09:58:50 +0200 Subject: [PATCH 3/4] lxc: avoid NULL dereference when we find no mount point
* src/lxc_container.c (lxcContainerUnmountOldFS): Don't pass a NULL pointer to qsort. --- src/lxc_container.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/lxc_container.c b/src/lxc_container.c index 950dd50..2073864 100644 --- a/src/lxc_container.c +++ b/src/lxc_container.c @@ -546,8 +546,9 @@ static int lxcContainerUnmountOldFS(void) } endmntent(procmnt);
- qsort(mounts, nmounts, sizeof(mounts[0]), - lxcContainerChildMountSort); + if (mounts) + qsort(mounts, nmounts, sizeof(mounts[0]), + lxcContainerChildMountSort);
for (i = 0 ; i < nmounts ; i++) { VIR_DEBUG("Umount %s", mounts[i]);
This would is impossible to hit, since you must at least have a /proc filesystem if we've got this far, but doesn't hurt to check anyway :-)
It can be triggered when the first getmntent call fails. I'll revise the log to mention that.
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Jim Meyering