On Thu, Sep 20, 2018 at 12:32:23PM +0200, David Hildenbrand wrote: > Although unlikely in practice, we could have integer overflows on some > calculations based on addresses and sizes, leading to error checks not > triggering. > > Let's properly handle this whenever we do an addition. Make > address_space_end point at the real end, instead of end + 1, so we don't > have to handle special cases like it being 0. This will allow us to > place a memory device at the very end of the guest physical 64bit address > space (if ever possible). > > Also, QEMU_ALIGN_UP(md_addr + md_size, align) could (theoretically) wrap to > address 0, so add a final check for 0. > > Reported-by: Dr. David Alan Gilbert <dgilb...@redhat.com> > Signed-off-by: David Hildenbrand <da...@redhat.com>
Reviewed-by: David Gibson <da...@gibson.dropbear.id.au>
> ---
> hw/mem/memory-device.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
> index efacbc2a7d..020aad102a 100644
> --- a/hw/mem/memory-device.c
> +++ b/hw/mem/memory-device.c
> @@ -85,7 +85,8 @@ static void memory_device_check_addable(MachineState *ms,
> uint64_t size,
>
> /* will we exceed the total amount of memory specified */
> memory_device_used_region_size(OBJECT(ms), &used_region_size);
> - if (used_region_size + size > ms->maxram_size - ms->ram_size) {
> + if (used_region_size + size < used_region_size ||
> + used_region_size + size > ms->maxram_size - ms->ram_size) {
> error_setg(errp, "not enough space, currently 0x%" PRIx64
> " in use of total hot pluggable 0x" RAM_ADDR_FMT,
> used_region_size, ms->maxram_size - ms->ram_size);
> @@ -115,7 +116,7 @@ uint64_t memory_device_get_free_addr(MachineState *ms,
> const uint64_t *hint,
> }
> address_space_start = ms->device_memory->base;
> address_space_end = address_space_start +
> - memory_region_size(&ms->device_memory->mr);
> + memory_region_size(&ms->device_memory->mr) - 1;
> g_assert(address_space_end >= address_space_start);
>
> /* address_space_start indicates the maximum alignment we expect */
> @@ -149,7 +150,8 @@ uint64_t memory_device_get_free_addr(MachineState *ms,
> const uint64_t *hint,
> "] before 0x%" PRIx64, new_addr, size,
> address_space_start);
> return 0;
> - } else if ((new_addr + size) > address_space_end) {
> + } else if (new_addr + size - 1 < new_addr ||
> + new_addr + size - 1 > address_space_end) {
> error_setg(errp, "can't add memory [0x%" PRIx64 ":0x%" PRIx64
> "] beyond 0x%" PRIx64, new_addr, size,
> address_space_end);
> @@ -182,7 +184,8 @@ uint64_t memory_device_get_free_addr(MachineState *ms,
> const uint64_t *hint,
> }
> }
>
> - if (new_addr + size > address_space_end) {
> + if (new_addr + size - 1 < new_addr || !new_addr ||
> + new_addr + size - 1 > address_space_end) {
> error_setg(errp, "could not find position in guest address space for
> "
> "memory device - memory fragmented due to alignments");
> goto out;
--
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
signature.asc
Description: PGP signature
