[libvirt] [Resending] [PATCH 2/2] Allow x86 to fetch sysinfo from /proc/cpuinfo when dmidecode is absent.

Eric Blake eblake at redhat.com
Tue Feb 28 23:31:01 UTC 2012


On 02/28/2012 12:58 PM, Daniel P. Berrange wrote:
> On Fri, Feb 17, 2012 at 01:00:22PM +0530, Prerna wrote:
>> The last patch was mangled by my mailer, resending.
>>
>> From: Prerna Saxena <prerna at linux.vnet.ibm.com>
>> Date: Thu, 16 Feb 2012 15:33:43 +0530
>> Subject: [PATCH 2/2] Sysinfo : Allow x86 to fetch sysinfo from /proc/cpuinfo
>>  in the event 'dmidecode' is absent in the system.
>>
>> Until now, libvirt on x86 flags an error message if dmidecode is not
>> found. With this patch, the following is a sample output on x86 when
>> dmidecode is absent:
>>

>> ---
>>  src/util/sysinfo.c |   95 +++++++++++++++++++++++++++++++++++++++++++++++++--
>>  1 files changed, 91 insertions(+), 4 deletions(-)
> 
> ACK

NACK; we need a v2 for both patches.  You had TABs (run 'make
syntax-check'), and this compiler warning is evidence of a real bug:

util/sysinfo.c: In function 'virSysinfoParseCPUInfoProcessor':
util/sysinfo.c:622:9: error: passing argument 1 of 'virSkipSpaces' from
incompatible pointer type [-Werror]
util/util.h:162:6: note: expected 'const char **' but argument is of
type 'char **'

Looking at that code:

        if ((eol) &&
            ((processor->processor_socket_destination =
                               strndup(cur, eol - cur)) == NULL))
            goto no_memory;
        virSkipSpaces(&(processor->processor_socket_destination));

but virSkipSpaces is _designed_ to advance the pointer.  Which means
that when you later call VIR_FREE on the altered pointer, you aren't
freeing the pointer returned by the malloc inside strndup, but are
instead freeing some random address occurring later in the allocated
chunk of memory - a surefire way to corrupt your heap.

I didn't compile-test the PowerPC code, but see the same bug there by
inspection.

If you want to skip spaces, you need to skip them in 'cur', prior to
calling strndup; something like:


diff --git i/src/util/sysinfo.c w/src/util/sysinfo.c
index de78d23..f8095e5 100644
--- i/src/util/sysinfo.c
+++ w/src/util/sysinfo.c
@@ -602,13 +602,15 @@ no_memory:
 static int
 virSysinfoParseCPUInfoProcessor(const char *base, virSysinfoDefPtr ret)
 {
-    char *cur, *eol, *tmp_base;
+    const char *cur;
+    char *eol, *tmp_base;
     virSysinfoProcessorDefPtr processor;

     while((tmp_base = strstr(base, "processor")) != NULL) {
         base = tmp_base;
         eol = strchr(base, '\n');
         cur = strchr(base, ':') + 1;
+        virSkipSpaces(&cur);

         if (VIR_EXPAND_N(ret->processor, ret->nprocessor, 1) < 0) {
             goto no_memory;
@@ -619,7 +621,6 @@ virSysinfoParseCPUInfoProcessor(const char *base,
virSysinfoDefPtr ret)
             ((processor->processor_socket_destination =
                                strndup(cur, eol - cur)) == NULL))
             goto no_memory;
-        virSkipSpaces(&(processor->processor_socket_destination));

         if ((cur = strstr(base, "vendor_id")) != NULL) {
             cur = strchr(cur, ':') + 1;

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120228/1461dcce/attachment-0001.sig>


More information about the libvir-list mailing list