Hello Cedric,
+-- On Wed, 24 Oct 2018, Cédric Le Goater wrote --+
| I think using a data[8] would be more appropriate. It would make the
| pnv_lpc_do_eccb() routine a little more complex. I tried to rewrite it to
| have a common one with the P9 LPC model but could not find a common pattern.
| P9 is purely MMIO based. Something on the TODO list.
|
| 8 bytes accesses will then fail anyhow because all MemoryRegionOps have a
| max_access_size = 4.
Thank you for these inputs, appreciate it. To confirm,
- You plan to send a v2 patch to fix the OOB access issue along with
refactoring pnv_lpc_do_eccb? If yes, kindly include me in CC please.
OR
- While we refactor the routine for better, a patch below seem okay to fix
the OOB access issue?
===
diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
index d7721320a2..655d5f3d07 100644
--- a/hw/ppc/pnv_lpc.c
+++ b/hw/ppc/pnv_lpc.c
@@ -155,9 +155,13 @@ static void pnv_lpc_do_eccb(PnvLpcController *lpc,
uint64_t cmd)
/* XXX Check for magic bits at the top, addr size etc... */
unsigned int sz = (cmd & ECCB_CTL_SZ_MASK) >> ECCB_CTL_SZ_LSH;
uint32_t opb_addr = cmd & ECCB_CTL_ADDR_MASK;
- uint8_t data[4];
+ uint8_t data[8];
bool success;
+ if (sz > sizeof(data)) {
+ return;
+ }
+
if (cmd & ECCB_CTL_READ) {
success = opb_read(lpc, opb_addr, data, sz);
if (success) {
===
Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
