Re: [libvirt] [PATCH] Fix closedir usage in virNumaGetPages
On 06/21/2014 05:29 PM, Roman Bogorodskiy wrote: virNumaGetPages calls closedir(dir) in cleanup and dir could be NULL if we jump there from the failed opendir() call. While it's not harmful on Linux, FreeBSD libc crashes [1], so make sure that dir is not NULL before calling closedir. 1: http://lists.freebsd.org/pipermail/freebsd-standards/2014-January/002704.html --- src/util/virnuma.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) ACK Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix closedir usage in virNumaGetPages
On 21.06.2014 17:29, Roman Bogorodskiy wrote: virNumaGetPages calls closedir(dir) in cleanup and dir could be NULL if we jump there from the failed opendir() call. While it's not harmful on Linux, FreeBSD libc crashes [1], so make sure that dir is not NULL before calling closedir. 1: http://lists.freebsd.org/pipermail/freebsd-standards/2014-January/002704.html --- src/util/virnuma.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/util/virnuma.c b/src/util/virnuma.c index c8e7f40..1048033 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -836,7 +836,8 @@ virNumaGetPages(int node, VIR_FREE(tmp_free); VIR_FREE(tmp_avail); VIR_FREE(tmp_size); -closedir(dir); +if (dir) +closedir(dir); VIR_FREE(path); return ret; } So why is free(NULL) safe on FreeBSD then? I'd call this a libc bug not a libvirt one. But since even we already have such borken design (remember our publir vir*Free() APIs?) I can live with this patch. ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix closedir usage in virNumaGetPages
On 06/23/2014 05:46 AM, Michal Privoznik wrote: -closedir(dir); +if (dir) +closedir(dir); VIR_FREE(path); return ret; } So why is free(NULL) safe on FreeBSD then? I'd call this a libc bug not a libvirt one. But since even we already have such borken design (remember our publir vir*Free() APIs?) I can live with this patch. free(NULL) is explicitly required by C (and therefore POSIX) to be safe. closedir(NULL) is intentionally unspecified by POSIX, and therefore unsafe. It would be nice if the two had similar requirements, but as POSIX was merely standardizing existing practice, we are stuck with them being different in behavior. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix closedir usage in virNumaGetPages
Eric Blake wrote: On 06/23/2014 05:46 AM, Michal Privoznik wrote: -closedir(dir); +if (dir) +closedir(dir); VIR_FREE(path); return ret; } So why is free(NULL) safe on FreeBSD then? I'd call this a libc bug not a libvirt one. But since even we already have such borken design (remember our publir vir*Free() APIs?) I can live with this patch. free(NULL) is explicitly required by C (and therefore POSIX) to be safe. closedir(NULL) is intentionally unspecified by POSIX, and therefore unsafe. It would be nice if the two had similar requirements, but as POSIX was merely standardizing existing practice, we are stuck with them being different in behavior. The patch is pushed now; thanks. Roman Bogorodskiy pgpJaW8oVBxxu.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix closedir usage in virNumaGetPages
virNumaGetPages calls closedir(dir) in cleanup and dir could be NULL if we jump there from the failed opendir() call. While it's not harmful on Linux, FreeBSD libc crashes [1], so make sure that dir is not NULL before calling closedir. 1: http://lists.freebsd.org/pipermail/freebsd-standards/2014-January/002704.html --- src/util/virnuma.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/util/virnuma.c b/src/util/virnuma.c index c8e7f40..1048033 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -836,7 +836,8 @@ virNumaGetPages(int node, VIR_FREE(tmp_free); VIR_FREE(tmp_avail); VIR_FREE(tmp_size); -closedir(dir); +if (dir) +closedir(dir); VIR_FREE(path); return ret; } -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix closedir usage in virNumaGetPages
On Sat, Jun 21, 2014 at 8:59 PM, Roman Bogorodskiy bogorods...@gmail.com wrote: virNumaGetPages calls closedir(dir) in cleanup and dir could be NULL if we jump there from the failed opendir() call. While it's not harmful on Linux, FreeBSD libc crashes [1], so make sure that dir is not NULL before calling closedir. 1: http://lists.freebsd.org/pipermail/freebsd-standards/2014-January/002704.html --- src/util/virnuma.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/util/virnuma.c b/src/util/virnuma.c index c8e7f40..1048033 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -836,7 +836,8 @@ virNumaGetPages(int node, VIR_FREE(tmp_free); VIR_FREE(tmp_avail); VIR_FREE(tmp_size); -closedir(dir); +if (dir) +closedir(dir); VIR_FREE(path); return ret; } -- 1.9.0 ACK --- Nehal J Wani -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
