| Message ID | 20190213172926.21740-1-farosas@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | spapr: fix out of bounds write in spapr_populate_drmem_v2 | expand |
On Wed, 13 Feb 2019 15:29:26 -0200 Fabiano Rosas <farosas@linux.ibm.com> wrote: > buf_len is uint8_t which is not large enough to hold the result of: > > nr_entries * sizeof(struct sPAPRDrconfCellV2) + sizeof(uint32_t); > > for a nr_entries greater than 10. > > This causes the allocated buffer 'int_buf' to be smaller than expected > and we eventually overwrite some of glibc's control structures (see > "chunk" in https://sourceware.org/glibc/wiki/MallocInternals) > > The following error is seen while trying to free int_buf: > > "free(): invalid next size (fast)" > > Fixes: a324d6f166 "spapr: Support ibm,dynamic-memory-v2 property" > Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> > --- Reviewed-by: Greg Kurz <groug@kaod.org> > hw/ppc/spapr.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 332cba89d4..7c4bf8ed8c 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -687,14 +687,14 @@ static int spapr_populate_drmem_v2(sPAPRMachineState *spapr, void *fdt, > int offset, MemoryDeviceInfoList *dimms) > { > MachineState *machine = MACHINE(spapr); > - uint8_t *int_buf, *cur_index, buf_len; > + uint8_t *int_buf, *cur_index; > int ret; > uint64_t lmb_size = SPAPR_MEMORY_BLOCK_SIZE; > uint64_t addr, cur_addr, size; > uint32_t nr_boot_lmbs = (machine->device_memory->base / lmb_size); > uint64_t mem_end = machine->device_memory->base + > memory_region_size(&machine->device_memory->mr); > - uint32_t node, nr_entries = 0; > + uint32_t node, buf_len, nr_entries = 0; > sPAPRDRConnector *drc; > DrconfCellQueue *elem, *next; > MemoryDeviceInfoList *info;
On Wed, Feb 13, 2019 at 03:29:26PM -0200, Fabiano Rosas wrote: > buf_len is uint8_t which is not large enough to hold the result of: > > nr_entries * sizeof(struct sPAPRDrconfCellV2) + sizeof(uint32_t); > > for a nr_entries greater than 10. > > This causes the allocated buffer 'int_buf' to be smaller than expected > and we eventually overwrite some of glibc's control structures (see > "chunk" in https://sourceware.org/glibc/wiki/MallocInternals) > > The following error is seen while trying to free int_buf: > > "free(): invalid next size (fast)" > > Fixes: a324d6f166 "spapr: Support ibm,dynamic-memory-v2 property" > Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> Ouch. That was always bogus, we just got away with it before. Applied to ppc-for-4.0. > --- > hw/ppc/spapr.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 332cba89d4..7c4bf8ed8c 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -687,14 +687,14 @@ static int spapr_populate_drmem_v2(sPAPRMachineState *spapr, void *fdt, > int offset, MemoryDeviceInfoList *dimms) > { > MachineState *machine = MACHINE(spapr); > - uint8_t *int_buf, *cur_index, buf_len; > + uint8_t *int_buf, *cur_index; > int ret; > uint64_t lmb_size = SPAPR_MEMORY_BLOCK_SIZE; > uint64_t addr, cur_addr, size; > uint32_t nr_boot_lmbs = (machine->device_memory->base / lmb_size); > uint64_t mem_end = machine->device_memory->base + > memory_region_size(&machine->device_memory->mr); > - uint32_t node, nr_entries = 0; > + uint32_t node, buf_len, nr_entries = 0; > sPAPRDRConnector *drc; > DrconfCellQueue *elem, *next; > MemoryDeviceInfoList *info;
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 332cba89d4..7c4bf8ed8c 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -687,14 +687,14 @@ static int spapr_populate_drmem_v2(sPAPRMachineState *spapr, void *fdt, int offset, MemoryDeviceInfoList *dimms) { MachineState *machine = MACHINE(spapr); - uint8_t *int_buf, *cur_index, buf_len; + uint8_t *int_buf, *cur_index; int ret; uint64_t lmb_size = SPAPR_MEMORY_BLOCK_SIZE; uint64_t addr, cur_addr, size; uint32_t nr_boot_lmbs = (machine->device_memory->base / lmb_size); uint64_t mem_end = machine->device_memory->base + memory_region_size(&machine->device_memory->mr); - uint32_t node, nr_entries = 0; + uint32_t node, buf_len, nr_entries = 0; sPAPRDRConnector *drc; DrconfCellQueue *elem, *next; MemoryDeviceInfoList *info;
buf_len is uint8_t which is not large enough to hold the result of: nr_entries * sizeof(struct sPAPRDrconfCellV2) + sizeof(uint32_t); for a nr_entries greater than 10. This causes the allocated buffer 'int_buf' to be smaller than expected and we eventually overwrite some of glibc's control structures (see "chunk" in https://sourceware.org/glibc/wiki/MallocInternals) The following error is seen while trying to free int_buf: "free(): invalid next size (fast)" Fixes: a324d6f166 "spapr: Support ibm,dynamic-memory-v2 property" Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> --- hw/ppc/spapr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)