Re: [Qemu-devel] [PATCH v6 08/26] RISC-V: Make sure rom has space for fdt

[Michael Clark]

On Sat, Mar 24, 2018 at 2:25 PM, Peter Maydell <address@hidden>
wrote:

> On 24 March 2018 at 18:13, Michael Clark <address@hidden> wrote:
> > Remove a potential buffer overflow (not seen in practice).
> > Perhaps cpu_physical_memory_write already has bound checks.
>
> cpu_physical_memory_write() writes to the guest address
> space, so it won't overflow. If you ask it to write
> off the end of a ROM then it will correctly write into
> an unassigned part of the guest memory space (which does
> nothing) or into whatever device or other ram is there.
> You probably don't want to do that, but it is not a buffer
> overflow.
>

I assumed that was the case but it is still probably good discipline to
have the bounds check.

We have also expanded the ROM regions to account for the default FDT size
which was larger than the previous ROM region sizes. I discovered this
while debugging another issue, where I had a debug statement to print the
fdt_size and noticed it was larger than the ROM region reserved for it.

It's belts and braces change. I'd prefer we at least make sure our ROM
regions are large enough for the default FDT size. It could be overflowed
on the virt board eventually if we enable many CPUs and add more devices.
The error message is a nice to have, as we'll know if the FDT size is too
large rather than have a subtle failure due to the boot loader parsing
truncated device tree.

This problem is not seen in practice... yet... but I still think it is
worth fixing.