From: Gonglei <arei.gonglei@huawei.com>
Date: Thu, 20 Nov 2014 11:35:02 +0000 (+0800)
Subject: pcnet: fix Negative array index read
X-Git-Tag: v2.2.0-rc3~5^2~1
X-Git-Url: http://git.qemu.org/?p=qemu.git;a=commitdiff_plain;h=7b50d00911ddd6d56a766ac5671e47304c20a21b

pcnet: fix Negative array index read

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.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Jason Wang <jasowang@redhat.com>
Reviewed-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---

diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
index d344c15..f409b92 100644
--- a/hw/net/pcnet.c
+++ b/hw/net/pcnet.c
@@ -1212,7 +1212,7 @@ static void pcnet_transmit(PCNetState *s)
     hwaddr xmit_cxda = 0;
     int count = CSR_XMTRL(s)-1;
     int add_crc = 0;
-
+    int bcnt;
     s->xmit_pos = -1;
 
     if (!CSR_TXON(s)) {
@@ -1247,35 +1247,40 @@ static void pcnet_transmit(PCNetState *s)
             s->xmit_pos = -1;
             goto txdone;
         }
+
+        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(qemu_get_queue(s->nic), s->buffer, s->xmit_pos);
-                s->looptest = 0;
-            } else
-                if (s->nic)
-                    qemu_send_packet(qemu_get_queue(s->nic), 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(qemu_get_queue(s->nic), s->buffer, s->xmit_pos);
+            s->looptest = 0;
+        } else {
+            if (s->nic) {
+                qemu_send_packet(qemu_get_queue(s->nic), 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)));
From: Petr Matousek <pmatouse@redhat.com>
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.

[Fixed 3-space indentation to QEMU's 4-space coding standard.
--Stefan]

Signed-off-by: Petr Matousek <pmatouse@redhat.com>
Reported-by: Matt Tait <matttait@google.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
(cherry picked from commit 9f7c594c006289ad41169b854d70f5da6e400a2a)
---
 hw/net/pcnet.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
index b18ac6c..455845b 100644
--- a/hw/net/pcnet.c
+++ b/hw/net/pcnet.c
@@ -1253,6 +1253,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;