On Wed, Oct 27, 2021 at 12:03 AM Damien Hedde <[email protected]> wrote: > > When loading a binary file, we only check if it is smaller than the > ram_size. It does not really check if the file will be loaded at an > existing location (if there is nothing at the target address, it will > "fail" silently later). It prevents loading a binary blob bigger than > ram_size too even if the target location is big enough. > > Replace this check by looking for the target memory region size and > prevent loading a bigger file than the available space. > > Get rid of "hw/boards.h" include, since we needed it only to access > `current_machine`. > > Signed-off-by: Damien Hedde <[email protected]>
Reviewed-by: Alistair Francis <[email protected]> Alistair > --- > > Hi, > > This is an updated version implementing what we discussed in v1. > > This can be tested easily, eg, using opentitan machine which has a 64K ram > located at 0x10000000. > > the following works (we a blob corresponding to the whole ram) > | $ dd bs=1K count=64 if=/dev/zero of=blob.bin > | $ qemu-system-riscv32 -display none -M opentitan -device > loader,addr=0x10000000,file=blob.bin > > but this command fails because we load a blob which is too big > | $ dd bs=1K count=64 if=/dev/zero of=blob.bin > | $ qemu-system-riscv32 -display none -M opentitan -device > loader,addr=0x10001000,file=blob.bin > | qemu-system-riscv32: -device loader,addr=0x10001000,file=blob.bin: Cannot > load specified image blob.bin > > and this command fails too (we load a blob at an unmapped location) > | $ dd bs=1K count=64 if=/dev/zero of=blob.bin > | $ qemu-system-riscv32 -display none -M opentitan -device > loader,addr=0x0,file=blob.bin > | qemu-system-riscv32: -device loader,addr=0x0,file=blob.bin: Address 0x0 > does not exists > > Thanks, > Damien > > v2: > + instead of disabling the ram_size check, look for the target > > v1: https://lists.nongnu.org/archive/html/qemu-devel/2021-10/msg01077.html > > See also the original discussion about generic-loader: > https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg04668.html > https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg04681.html > --- > hw/core/generic-loader.c | 20 +++++++++++++++++--- > 1 file changed, 17 insertions(+), 3 deletions(-) > > diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c > index d14f932eea..88d3f9fd56 100644 > --- a/hw/core/generic-loader.c > +++ b/hw/core/generic-loader.c > @@ -34,7 +34,6 @@ > #include "hw/core/cpu.h" > #include "sysemu/dma.h" > #include "sysemu/reset.h" > -#include "hw/boards.h" > #include "hw/loader.h" > #include "hw/qdev-properties.h" > #include "qapi/error.h" > @@ -153,8 +152,23 @@ static void generic_loader_realize(DeviceState *dev, > Error **errp) > } > > if (size < 0 || s->force_raw) { > - /* Default to the maximum size being the machine's ram size */ > - size = load_image_targphys_as(s->file, s->addr, > current_machine->ram_size, as); > + MemoryRegion *root = as ? as->root : get_system_memory(); > + MemoryRegionSection mrs; > + uint64_t avail = 0; > + > + mrs = memory_region_find(root, s->addr, 1); > + > + if (mrs.mr) { > + avail = int128_get64(mrs.mr->size) - > mrs.offset_within_region; > + memory_region_unref(mrs.mr); > + } else { > + error_setg(errp, "Address 0x%" PRIx64 " does not exists", > + s->addr); > + return; > + } > + > + /* Limit the file size to the memory region space */ > + size = load_image_targphys_as(s->file, s->addr, avail, as); > } else { > s->addr = entry; > } > -- > 2.33.0 >
