[Qemu-devel] [PATCH] ppc/spapr: Fix buffer overflow in spapr_populate_drconf_memory()

[Qemu-devel] [PATCH] ppc/spapr: Fix buffer overflow in spapr_populate_drconf_memory()

[Thomas Huth]

The buffer that is allocated in spapr_populate_drconf_memory()
is used for setting both, the "ibm,dynamic-memory" and the
"ibm,associativity-lookup-arrays" property. However, only the
size of the first one is taken into account when allocating the
memory. So if the length of the second property is larger than
the length of the first one, we run into a buffer overflow here!
Fix it by taking the length of the second property into account,
too.

Fixes: "spapr: Support ibm,dynamic-reconfiguration-memory" patch
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 Note: This is for the spapr-next branch only, the patch
 which introduces this problem is not on master yet.

 hw/ppc/spapr.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index f22db12..e4177fb 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -725,9 +725,12 @@ static int spapr_populate_drconf_memory(sPAPRMachineState *spapr, void *fdt)
     uint32_t *int_buf, *cur_index, buf_len;
     int nr_nodes = nb_numa_nodes ? nb_numa_nodes : 1;
 
-    /* Allocate enough buffer size to fit in ibm,dynamic-memory */
-    buf_len = nr_lmbs * SPAPR_DR_LMB_LIST_ENTRY_SIZE * sizeof(uint32_t) +
-                sizeof(uint32_t);
+    /*
+     * Allocate enough buffer size to fit in ibm,dynamic-memory
+     * or ibm,associativity-lookup-arrays
+     */
+    buf_len = MAX(nr_lmbs * SPAPR_DR_LMB_LIST_ENTRY_SIZE + 1, nr_nodes * 4 + 2)
+              * sizeof(uint32_t);
     cur_index = int_buf = g_malloc0(buf_len);
 
     offset = fdt_add_subnode(fdt, 0, "ibm,dynamic-reconfiguration-memory");
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] ppc/spapr: Fix buffer overflow in spapr_populate_drconf_memory()

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 882 bytes --]

On Tue, Sep 15, 2015 at 09:34:20PM +0200, Thomas Huth wrote:
> The buffer that is allocated in spapr_populate_drconf_memory()
> is used for setting both, the "ibm,dynamic-memory" and the
> "ibm,associativity-lookup-arrays" property. However, only the
> size of the first one is taken into account when allocating the
> memory. So if the length of the second property is larger than
> the length of the first one, we run into a buffer overflow here!
> Fix it by taking the length of the second property into account,
> too.
> 
> Fixes: "spapr: Support ibm,dynamic-reconfiguration-memory" patch
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Merged to spapr-next, thanks.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]