| Message ID | 1438270422-18018-1-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On 30 July 2015 at 23:02, Aurelien Jarno <aurelien@aurel32.net> wrote: > On 2015-07-30 16:33, Peter Maydell wrote: >> The LDMA and COP memory regions represent four 32 bit registers >> each, but the memory regions themselves are 0x100 bytes large. >> Add guards to the read and write accessors so that bogus accesses >> beyond the four defined registers don't just run off the end of >> the bonldma and boncop structs and into whatever lies beyond. > > Thanks for finding that. I don't know if it is better to reduce the > memory region or just ignore the access as in your patch. I haven't > found any documentation about the bonito northbridge, so I think it's > safer to go like in your patch. I did find some documentation by random googling -- but it just defines that there are four valid registers in each region, and doesn't say anything about what happens in the gaps in between... > I have just tested, it still boots fine with the change. > >> hw/pci-host/bonito.c | 16 ++++++++++++++++ >> 1 file changed, 16 insertions(+) > > Acked-by: Aurelien Jarno <aurelien@aurel32.net> Thanks. (I haven't marked this as for-2.4 because it's been like this since forever, and fulong2e isn't a KVM board we care about security on; this is just a random cleanup I happened to remember about. I could be persuaded that it ought to go in, though.) -- PMM
diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c index 3a731fe..4139a2c 100644 --- a/hw/pci-host/bonito.c +++ b/hw/pci-host/bonito.c @@ -355,6 +355,10 @@ static uint64_t bonito_ldma_readl(void *opaque, hwaddr addr, uint32_t val; PCIBonitoState *s = opaque; + if (addr >= sizeof(s->bonldma)) { + return 0; + } + val = ((uint32_t *)(&s->bonldma))[addr/sizeof(uint32_t)]; return val; @@ -365,6 +369,10 @@ static void bonito_ldma_writel(void *opaque, hwaddr addr, { PCIBonitoState *s = opaque; + if (addr >= sizeof(s->bonldma)) { + return; + } + ((uint32_t *)(&s->bonldma))[addr/sizeof(uint32_t)] = val & 0xffffffff; } @@ -384,6 +392,10 @@ static uint64_t bonito_cop_readl(void *opaque, hwaddr addr, uint32_t val; PCIBonitoState *s = opaque; + if (addr >= sizeof(s->boncop)) { + return 0; + } + val = ((uint32_t *)(&s->boncop))[addr/sizeof(uint32_t)]; return val; @@ -394,6 +406,10 @@ static void bonito_cop_writel(void *opaque, hwaddr addr, { PCIBonitoState *s = opaque; + if (addr >= sizeof(s->boncop)) { + return; + } + ((uint32_t *)(&s->boncop))[addr/sizeof(uint32_t)] = val & 0xffffffff; }
The LDMA and COP memory regions represent four 32 bit registers each, but the memory regions themselves are 0x100 bytes large. Add guards to the read and write accessors so that bogus accesses beyond the four defined registers don't just run off the end of the bonldma and boncop structs and into whatever lies beyond. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- I don't have a fulong2e image, so this is compile tested only... hw/pci-host/bonito.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)