[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH] x86Decode: avoid NULL-dereference upon questionable input



Clang warned about the potential NULL-dereference
via the STREQ/strcmp below.  models[i] could be NULL.
Even "models" could be NULL, and the "allowed = ..." test
makes that appear to be deliberately allowed.

The change below is the least invasive and cleanest
I could come up with, assuming there is no need to diagnose
(e.g., by returning -1) the condition of a NULL models[i] pointer.

>From 623ac546a93f3e5b554647b05524b5041a642530 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering redhat com>
Date: Sun, 28 Feb 2010 13:34:06 +0100
Subject: [PATCH] x86Decode: avoid NULL-dereference upon questionable input

* src/cpu/cpu_x86.c (x86Decode): Don't dereference NULL when passed
a NULL "models" pointer, or when passed a nonzero "nmodels" value
and a corresponding NULL models[i].
---
 src/cpu/cpu_x86.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 2194c32..b263629 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -957,89 +957,89 @@ x86Compare(virCPUDefPtr host,
            virCPUDefPtr cpu)
 {
     return x86Compute(host, cpu, NULL);
 }


 static virCPUCompareResult
 x86GuestData(virCPUDefPtr host,
              virCPUDefPtr guest,
              union cpuData **data)
 {
     return x86Compute(host, guest, data);
 }


 static int
 x86Decode(virCPUDefPtr cpu,
           const union cpuData *data,
           const char **models,
           unsigned int nmodels)
 {
     int ret = -1;
     struct x86_map *map;
     const struct x86_model *candidate;
     virCPUDefPtr cpuCandidate;
     virCPUDefPtr cpuModel = NULL;
     struct cpuX86cpuid *cpuid;
     int i;

     if (data == NULL || (map = x86LoadMap()) == NULL)
         return -1;

     candidate = map->models;
     while (candidate != NULL) {
         bool allowed = (models == NULL);

         for (i = 0; i < candidate->ncpuid; i++) {
             cpuid = x86DataCpuid(data, candidate->cpuid[i].function);
             if (cpuid == NULL
                 || !x86cpuidMatchMasked(cpuid, candidate->cpuid + i))
                 goto next;
         }

         for (i = 0; i < nmodels; i++) {
-            if (STREQ(models[i], candidate->name)) {
+            if (models && models[i] && STREQ(models[i], candidate->name)) {
                 allowed = true;
                 break;
             }
         }

         if (!allowed) {
             VIR_DEBUG("CPU model %s not allowed by hypervisor; ignoring",
                       candidate->name);
             goto next;
         }

         if (!(cpuCandidate = x86DataToCPU(data, candidate, map)))
             goto out;

         if (cpuModel == NULL
             || cpuModel->nfeatures > cpuCandidate->nfeatures) {
             virCPUDefFree(cpuModel);
             cpuModel = cpuCandidate;
         } else
             virCPUDefFree(cpuCandidate);

     next:
         candidate = candidate->next;
     }

     if (cpuModel == NULL) {
         virCPUReportError(VIR_ERR_INTERNAL_ERROR,
                 "%s", _("Cannot find suitable CPU model for given data"));
         goto out;
     }

     cpu->model = cpuModel->model;
     cpu->nfeatures = cpuModel->nfeatures;
     cpu->features = cpuModel->features;
     VIR_FREE(cpuModel);

     ret = 0;

 out:
     x86MapFree(map);
     virCPUDefFree(cpuModel);

     return ret;
 }
--
1.7.0.1.414.g89213d


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]