Re: [Qemu-devel] [PATCH] lsi53c895a: check script ram address value

[Peter Maydell]

On 6 November 2018 at 11:53, P J P <address@hidden> wrote:
> From: Prasad J Pandit <address@hidden>
>
> While accessing script ram[2048] via 'lsi_ram_read/write' routines,
> 'addr' could exceed the ram range. Mask high order bits to avoid
> OOB access.
>
> Reported-by: Mark Kanda <address@hidden>
> Signed-off-by: Prasad J Pandit <address@hidden>
> ---
>  hw/scsi/lsi53c895a.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> index 3f207f607c..0800df416e 100644
> --- a/hw/scsi/lsi53c895a.c
> +++ b/hw/scsi/lsi53c895a.c
> @@ -2035,6 +2035,7 @@ static void lsi_ram_write(void *opaque, hwaddr addr,
>      uint32_t mask;
>      int shift;
>
> +    addr &= 0x01FFF;
>      newval = s->script_ram[addr >> 2];
>      shift = (addr & 3) * 8;
>      mask = ((uint64_t)1 << (size * 8)) - 1;
> @@ -2050,6 +2051,7 @@ static uint64_t lsi_ram_read(void *opaque, hwaddr addr,
>      uint32_t val;
>      uint32_t mask;
>
> +    addr &= 0x01FFF;
>      val = s->script_ram[addr >> 2];
>      mask = ((uint64_t)1 << (size * 8)) - 1;
>      val >>= (addr & 3) * 8;
> --

When can this masking have any effect? These functions are
the read and write ops for lsi_ram_ops, which we register with
    memory_region_init_io(&s->ram_io, OBJECT(s), &lsi_ram_ops, s,
                          "lsi-ram", 0x2000);
which specifies a memory region size of 0x2000. So the input
addr must be in the 0..0x1fff range already -- or have I missed
something ?

It would probably be helpful (for readers and static analysers)
to assert() that addr is < 0x2000, though.

thanks
-- PMM