[libvirt] [PATCH] cpu_x86.c: avoid NULL-deref for invalid arguments

[Jiri Denemark]

> Passing a NULL "models" pointer along with a
> contradictory "nmodels >= 1" would cause a NULL-dereference.
> 
> An alternative to the fix below would be simply to guard
> the NULL-derferencing strcmp with "if (models ...",
> but that wouldn't tell the caller that they're passing
> bogus arguments.
...
> diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
> index dae7c90..47dc400 100644
> --- a/src/cpu/cpu_x86.c
> +++ b/src/cpu/cpu_x86.c
> @@ -1,7 +1,7 @@
>  /*
>   * cpu_x86.c: CPU driver for CPUs with x86 compatible CPUID instruction
>   *
> - * Copyright (C) 2009 Red Hat, Inc.
> + * Copyright (C) 2009-2010 Red Hat, Inc.
>   *
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
> @@ -954,6 +954,9 @@ x86Decode(virCPUDefPtr cpu,
>      if (data == NULL || (map = x86LoadMap()) == NULL)
>          return -1;
> 
> +    if (models == NULL && nmodels != 0)
> +        return -1;
> +

Hmm, this check introduces a possible memory leak, as it exists the function
without freeing map. We could just move the check at the beginning of the
function but since this is a private architecture specific implementation for
cpuDecode, I'd rather move the check one level up to the arch independent
entry point. A patch for that is attached.

Jirka
-------------- next part --------------
From 4e4c29a1198b5bb0af7cfaa666acd8b071f1b4c8 Mon Sep 17 00:00:00 2001
Message-Id: <4e4c29a1198b5bb0af7cfaa666acd8b071f1b4c8.1265024610.git.jdenemar at redhat.com>
From: Jiri Denemark <jdenemar at redhat.com>
Date: Mon, 1 Feb 2010 12:42:27 +0100
Subject: [PATCH] Move models/nmodels mismatch checking one level up
Mail-Followup-To: libvir-list at redhat.com

Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
---
 src/cpu/cpu.c     |    6 ++++++
 src/cpu/cpu_x86.c |    3 ---
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index 975ca28..3e46948 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -128,6 +128,12 @@ cpuDecode(virConnectPtr conn,
 {
     struct cpuArchDriver *driver;
 
+    if (models == NULL && nmodels != 0) {
+        virCPUReportError(conn, VIR_ERR_INTERNAL_ERROR,
+                "%s", _("nonzero nmodels doesn't match with NULL models"));
+        return -1;
+    }
+
     if (cpu == NULL) {
         virCPUReportError(conn, VIR_ERR_INTERNAL_ERROR,
                           "%s", _("invalid CPU definition"));
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 47dc400..ce55588 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -954,9 +954,6 @@ x86Decode(virCPUDefPtr cpu,
     if (data == NULL || (map = x86LoadMap()) == NULL)
         return -1;
 
-    if (models == NULL && nmodels != 0)
-        return -1;
-
     candidate = map->models;
     while (candidate != NULL) {
         bool allowed = (models == NULL);
-- 
1.6.6.1