$NetBSD: patch-XSA135,v 1.1 2015/06/23 17:45:33 bouyer Exp $ pcnet: fix Negative array index read From: Gonglei s->xmit_pos maybe assigned to a negative value (-1), but in this branch variable s->xmit_pos as an index to array s->buffer. Let's add a check for s->xmit_pos. upstream-commit-id: 7b50d00911ddd6d56a766ac5671e47304c20a21b Signed-off-by: Gonglei Signed-off-by: Paolo Bonzini Reviewed-by: Jason Wang Reviewed-by: Jason Wang Signed-off-by: Stefan Hajnoczi diff --git a/hw/pcnet.c b/hw/pcnet.c index 7cc0637..9f3e1cc 100644 --- qemu-xen-traditional/hw/pcnet.c.orig +++ qemu-xen-traditional/hw/pcnet.c @@ -1250,7 +1250,7 @@ static void pcnet_transmit(PCNetState *s) target_phys_addr_t xmit_cxda = 0; int count = CSR_XMTRL(s)-1; int add_crc = 0; - + int bcnt; s->xmit_pos = -1; if (!CSR_TXON(s)) { @@ -1276,34 +1276,39 @@ static void pcnet_transmit(PCNetState *s) if (BCR_SWSTYLE(s) != 1) add_crc = GET_FIELD(tmd.status, TMDS, ADDFCS); } + + if (s->xmit_pos < 0) { + goto txdone; + } + + bcnt = 4096 - GET_FIELD(tmd.length, TMDL, BCNT); + s->phys_mem_read(s->dma_opaque, PHYSADDR(s, tmd.tbadr), + s->buffer + s->xmit_pos, bcnt, CSR_BSWP(s)); + s->xmit_pos += bcnt; + if (!GET_FIELD(tmd.status, TMDS, ENP)) { - int bcnt = 4096 - GET_FIELD(tmd.length, TMDL, BCNT); - s->phys_mem_read(s->dma_opaque, PHYSADDR(s, tmd.tbadr), - s->buffer + s->xmit_pos, bcnt, CSR_BSWP(s)); - s->xmit_pos += bcnt; - } else if (s->xmit_pos >= 0) { - int bcnt = 4096 - GET_FIELD(tmd.length, TMDL, BCNT); - s->phys_mem_read(s->dma_opaque, PHYSADDR(s, tmd.tbadr), - s->buffer + s->xmit_pos, bcnt, CSR_BSWP(s)); - s->xmit_pos += bcnt; + goto txdone; + } #ifdef PCNET_DEBUG - printf("pcnet_transmit size=%d\n", s->xmit_pos); + printf("pcnet_transmit size=%d\n", s->xmit_pos); #endif - if (CSR_LOOP(s)) { - if (BCR_SWSTYLE(s) == 1) - add_crc = !GET_FIELD(tmd.status, TMDS, NOFCS); - s->looptest = add_crc ? PCNET_LOOPTEST_CRC : PCNET_LOOPTEST_NOCRC; - pcnet_receive(s, s->buffer, s->xmit_pos); - s->looptest = 0; - } else - if (s->vc) - qemu_send_packet(s->vc, s->buffer, s->xmit_pos); - - s->csr[0] &= ~0x0008; /* clear TDMD */ - s->csr[4] |= 0x0004; /* set TXSTRT */ - s->xmit_pos = -1; + if (CSR_LOOP(s)) { + if (BCR_SWSTYLE(s) == 1) + add_crc = !GET_FIELD(tmd.status, TMDS, NOFCS); + s->looptest = add_crc ? PCNET_LOOPTEST_CRC : PCNET_LOOPTEST_NOCRC; + pcnet_receive(s, s->buffer, s->xmit_pos); + s->looptest = 0; + } else { + if (s->vc) { + qemu_send_packet(s->vc, s->buffer, s->xmit_pos); + } } + s->csr[0] &= ~0x0008; /* clear TDMD */ + s->csr[4] |= 0x0004; /* set TXSTRT */ + s->xmit_pos = -1; + + txdone: SET_FIELD(&tmd.status, TMDS, OWN, 0); TMDSTORE(&tmd, PHYSADDR(s,CSR_CXDA(s))); if (!CSR_TOKINTD(s) || (CSR_LTINTEN(s) && GET_FIELD(tmd.status, TMDS, LTINT))) From 2630672ab22255de252f877709851c0557a1c647 Mon Sep 17 00:00:00 2001 From: Petr Matousek Date: Sun, 24 May 2015 10:53:44 +0200 Subject: [PATCH] pcnet: force the buffer access to be in bounds during tx 4096 is the maximum length per TMD and it is also currently the size of the relay buffer pcnet driver uses for sending the packet data to QEMU for further processing. With packet spanning multiple TMDs it can happen that the overall packet size will be bigger than sizeof(buffer), which results in memory corruption. Fix this by only allowing to queue maximum sizeof(buffer) bytes. This is CVE-2015-3209. Signed-off-by: Petr Matousek Reported-by: Matt Tait Reviewed-by: Peter Maydell Reviewed-by: Stefan Hajnoczi --- hw/pcnet.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/hw/pcnet.c b/hw/pcnet.c index bdfd38f..6d32e4c 100644 --- qemu-xen-traditional/hw/pcnet.c.orig +++ qemu-xen-traditional/hw/pcnet.c @@ -1241,6 +1241,14 @@ static void pcnet_transmit(PCNetState *s) } bcnt = 4096 - GET_FIELD(tmd.length, TMDL, BCNT); + + /* if multi-tmd packet outsizes s->buffer then skip it silently. + Note: this is not what real hw does */ + if (s->xmit_pos + bcnt > sizeof(s->buffer)) { + s->xmit_pos = -1; + goto txdone; + } + s->phys_mem_read(s->dma_opaque, PHYSADDR(s, tmd.tbadr), s->buffer + s->xmit_pos, bcnt, CSR_BSWP(s)); s->xmit_pos += bcnt; -- 2.1.0