[Qemu-devel] [PULL V2 for 2.5 0/6] Net patches

[Qemu-devel] [PULL V2 for 2.5 0/6] Net patches

[Jason Wang]

The following changes since commit c012e1b7ad066f462ba1c3322fcb43cd8295eaff:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20151027-1' into staging (2015-10-27 16:17:55 +0000)

are available in the git repository at:

  https://github.com/jasowang/qemu.git tags/net-pull-request

for you to fetch changes up to 52b4bb7383b32e4e7512f98c57738c8fc9cb35ba:

  lan9118: log and ignore access to invalid registers, rather than aborting (2015-12-07 21:43:48 +0800)

----------------------------------------------------------------

Last minutes fixes for 2.5:

- Fix e1000 hang issue during win2k12 guest driver shutdown
- Fix two pcnet buffer overflow CVEs
- Fix lan9118 mac address loaded bit and warn instead of aborting
  when accessing unimplemented registers

Changes from V1:
- Unbreak 32bit build by dropping the vmxnet3 debug printf fixes

Signed-off-by: Jason Wang <jasowang@redhat.com>

----------------------------------------------------------------
Andrew Baumann (2):
      lan9118: fix emulation of MAC address loaded bit in E2P_CMD register
      lan9118: log and ignore access to invalid registers, rather than aborting

Denis V. Lunev (1):
      e1000: fix hang of win2k12 shutdown with flood ping

Jason Wang (1):
      pcnet: fix rx buffer overflow(CVE-2015-7512)

Michael S. Tsirkin (1):
      vmxnet3: silence warning

Prasad J Pandit (1):
      net: pcnet: add check to validate receive data size(CVE-2015-7504)

 hw/net/e1000.c   |  5 +++++
 hw/net/lan9118.c | 20 +++++++++++++-------
 hw/net/pcnet.c   | 14 +++++++++++---
 hw/net/vmxnet3.c |  1 -
 4 files changed, 29 insertions(+), 11 deletions(-)

-- 
2.5.0

[Qemu-devel] [PULL V2 for 2.5 1/6] e1000: fix hang of win2k12 shutdown with flood ping

[Jason Wang]

From: "Denis V. Lunev" <den@openvz.org>

e1000 driver in Win2k12 is really well rotten. It 100% hangs on shutdown
of UP VM under flood ping. The guest checks card state and reinjects
itself interrupt in a loop. This is fatal for UP machine.

There is no good way to fix this misbehavior but to kludge it. The
emulation has interrupt throttling register aka ITR which limits
interrupt rate and allows the guest to proceed this phase.
There is no problem with this kludge for Linux guests - it adjust the
value of it itself.

On the other hand according to the initial research in
    commit e9845f0985f088dd01790f4821026df0afba5795
    Author: Vincenzo Maffione <v.maffione@gmail.com>
    Date:   Fri Aug 2 18:30:52 2013 +0200

    e1000: add interrupt mitigation support

    ...

    Interrupt mitigation boosts performance when the guest suffers from
    an high interrupt rate (i.e. receiving short UDP packets at high packet
    rate). For some numerical results see the following link
    http://info.iet.unipi.it/~luigi/papers/20130520-rizzo-vm.pdf

this should also boost performance a bit.

See https://bugzilla.redhat.com/show_bug.cgi?id=874406 for additional
details.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Vincenzo Maffione <v.maffione@gmail.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/e1000.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 910de3a..1531244 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -426,6 +426,11 @@ static void e1000_reset(void *opaque)
         e1000_link_down(d);
     }
 
+    /* Throttle interrupts to prevent guest (e.g Win 2012) from
+     * reinjecting interrupts endlessly. TODO: fix non ITR case.
+     */
+    d->mac_reg[ITR] = 250;
+
     /* Some guests expect pre-initialized RAH/RAL (AddrValid flag + MACaddr) */
     d->mac_reg[RA] = 0;
     d->mac_reg[RA + 1] = E1000_RAH_AV;
-- 
2.5.0

[Qemu-devel] [PULL V2 for 2.5 2/6] net: pcnet: add check to validate receive data size(CVE-2015-7504)

[Jason Wang]

From: Prasad J Pandit <pjp@fedoraproject.org>

In loopback mode, pcnet_receive routine appends CRC code to the
receive buffer. If the data size given is same as the buffer size,
the appended CRC code overwrites 4 bytes after s->buffer. Added a
check to avoid that.

Reported by: Qinghao Tang <luodalongde@gmail.com>
Cc: qemu-stable@nongnu.org
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/pcnet.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
index 0eb3cc4..309c40b 100644
--- a/hw/net/pcnet.c
+++ b/hw/net/pcnet.c
@@ -1084,7 +1084,7 @@ ssize_t pcnet_receive(NetClientState *nc, const uint8_t *buf, size_t size_)
                 uint32_t fcs = ~0;
                 uint8_t *p = src;
 
-                while (p != &src[size-4])
+                while (p != &src[size])
                     CRC(fcs, *p++);
                 crc_err = (*(uint32_t *)p != htonl(fcs));
             }
@@ -1233,8 +1233,10 @@ 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)) {
+         * Note: this is not what real hw does.
+         * Last four bytes of s->buffer are used to store CRC FCS code.
+         */
+        if (s->xmit_pos + bcnt > sizeof(s->buffer) - 4) {
             s->xmit_pos = -1;
             goto txdone;
         }
-- 
2.5.0

[Qemu-devel] [PULL V2 for 2.5 3/6] pcnet: fix rx buffer overflow(CVE-2015-7512)

[Jason Wang]

Backends could provide a packet whose length is greater than buffer
size. Check for this and truncate the packet to avoid rx buffer
overflow in this case.

Cc: Prasad J Pandit <pjp@fedoraproject.org>
Cc: qemu-stable@nongnu.org
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/pcnet.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
index 309c40b..1f4a3db 100644
--- a/hw/net/pcnet.c
+++ b/hw/net/pcnet.c
@@ -1064,6 +1064,12 @@ ssize_t pcnet_receive(NetClientState *nc, const uint8_t *buf, size_t size_)
             int pktcount = 0;
 
             if (!s->looptest) {
+                if (size > 4092) {
+#ifdef PCNET_DEBUG_RMD
+                    fprintf(stderr, "pcnet: truncates rx packet.\n");
+#endif
+                    size = 4092;
+                }
                 memcpy(src, buf, size);
                 /* no need to compute the CRC */
                 src[size] = 0;
-- 
2.5.0

[Qemu-devel] [PULL V2 for 2.5 4/6] vmxnet3: silence warning

[Jason Wang]

From: "Michael S. Tsirkin" <mst@redhat.com>

vmxnet3 always produces a warning under qtest.

This is not a user error, don't warn.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/vmxnet3.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 5e3a233..37373e5 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -2015,7 +2015,6 @@ static bool vmxnet3_peer_has_vnet_hdr(VMXNET3State *s)
         return true;
     }
 
-    VMW_WRPRN("Peer has no virtio extension. Task offloads will be emulated.");
     return false;
 }
 
-- 
2.5.0

[Qemu-devel] [PULL V2 for 2.5 5/6] lan9118: fix emulation of MAC address loaded bit in E2P_CMD register

[Jason Wang]

From: Andrew Baumann <Andrew.Baumann@microsoft.com>

There appears to have been a longstanding typo in the implementation
of the "MAC address loaded" bit in the E2P_CMD (EEPROM command)
register. The code was using 0x10, but the controller spec says it
should be bit 8 (0x100).

Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/lan9118.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/net/lan9118.c b/hw/net/lan9118.c
index 4f0e840..133fd3d 100644
--- a/hw/net/lan9118.c
+++ b/hw/net/lan9118.c
@@ -56,6 +56,8 @@ do { fprintf(stderr, "lan9118: error: " fmt , ## __VA_ARGS__);} while (0)
 #define CSR_E2P_CMD     0xb0
 #define CSR_E2P_DATA    0xb4
 
+#define E2P_CMD_MAC_ADDR_LOADED 0x100
+
 /* IRQ_CFG */
 #define IRQ_INT         0x00001000
 #define IRQ_EN          0x00000100
@@ -352,14 +354,14 @@ static void lan9118_reload_eeprom(lan9118_state *s)
 {
     int i;
     if (s->eeprom[0] != 0xa5) {
-        s->e2p_cmd &= ~0x10;
+        s->e2p_cmd &= ~E2P_CMD_MAC_ADDR_LOADED;
         DPRINTF("MACADDR load failed\n");
         return;
     }
     for (i = 0; i < 6; i++) {
         s->conf.macaddr.a[i] = s->eeprom[i + 1];
     }
-    s->e2p_cmd |= 0x10;
+    s->e2p_cmd |= E2P_CMD_MAC_ADDR_LOADED;
     DPRINTF("MACADDR loaded from eeprom\n");
     lan9118_mac_changed(s);
 }
@@ -937,7 +939,7 @@ static uint32_t do_mac_read(lan9118_state *s, int reg)
 
 static void lan9118_eeprom_cmd(lan9118_state *s, int cmd, int addr)
 {
-    s->e2p_cmd = (s->e2p_cmd & 0x10) | (cmd << 28) | addr;
+    s->e2p_cmd = (s->e2p_cmd & E2P_CMD_MAC_ADDR_LOADED) | (cmd << 28) | addr;
     switch (cmd) {
     case 0:
         s->e2p_data = s->eeprom[addr];
-- 
2.5.0

[Qemu-devel] [PULL V2 for 2.5 6/6] lan9118: log and ignore access to invalid registers, rather than aborting

[Jason Wang]

From: Andrew Baumann <Andrew.Baumann@microsoft.com>

With this change, access to invalid/unimplemented device registers are
logged as a "guest error" rather than aborting qemu with
hw_error. This enables drivers for similar devices (e.g. SMSC 9221),
by simply ignoring the unimplemented writes. It's also closer to what
real hardware does.

Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/lan9118.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/net/lan9118.c b/hw/net/lan9118.c
index 133fd3d..1734b52 100644
--- a/hw/net/lan9118.c
+++ b/hw/net/lan9118.c
@@ -904,7 +904,8 @@ static void do_mac_write(lan9118_state *s, int reg, uint32_t val)
          */
         break;
     default:
-        hw_error("lan9118: Unimplemented MAC register write: %d = 0x%x\n",
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "lan9118: Unimplemented MAC register write: %d = 0x%x\n",
                  s->mac_cmd & 0xf, val);
     }
 }
@@ -932,8 +933,10 @@ static uint32_t do_mac_read(lan9118_state *s, int reg)
     case MAC_FLOW:
         return s->mac_flow;
     default:
-        hw_error("lan9118: Unimplemented MAC register read: %d\n",
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "lan9118: Unimplemented MAC register read: %d\n",
                  s->mac_cmd & 0xf);
+        return 0;
     }
 }
 
@@ -1130,7 +1133,8 @@ static void lan9118_writel(void *opaque, hwaddr offset,
         break;
 
     default:
-        hw_error("lan9118_write: Bad reg 0x%x = %x\n", (int)offset, (int)val);
+        qemu_log_mask(LOG_GUEST_ERROR, "lan9118_write: Bad reg 0x%x = %x\n",
+                      (int)offset, (int)val);
         break;
     }
     lan9118_update(s);
@@ -1248,7 +1252,7 @@ static uint64_t lan9118_readl(void *opaque, hwaddr offset,
     case CSR_E2P_DATA:
         return s->e2p_data;
     }
-    hw_error("lan9118_read: Bad reg 0x%x\n", (int)offset);
+    qemu_log_mask(LOG_GUEST_ERROR, "lan9118_read: Bad reg 0x%x\n", (int)offset);
     return 0;
 }
 
-- 
2.5.0

Re: [Qemu-devel] [PULL V2 for 2.5 0/6] Net patches

[Peter Maydell]

On 7 December 2015 at 14:17, Jason Wang <jasowang@redhat.com> wrote:
> The following changes since commit c012e1b7ad066f462ba1c3322fcb43cd8295eaff:
>
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20151027-1' into staging (2015-10-27 16:17:55 +0000)
>
> are available in the git repository at:
>
>   https://github.com/jasowang/qemu.git tags/net-pull-request
>
> for you to fetch changes up to 52b4bb7383b32e4e7512f98c57738c8fc9cb35ba:
>
>   lan9118: log and ignore access to invalid registers, rather than aborting (2015-12-07 21:43:48 +0800)
>
> ----------------------------------------------------------------
>
> Last minutes fixes for 2.5:
>
> - Fix e1000 hang issue during win2k12 guest driver shutdown
> - Fix two pcnet buffer overflow CVEs
> - Fix lan9118 mac address loaded bit and warn instead of aborting
>   when accessing unimplemented registers
>
> Changes from V1:
> - Unbreak 32bit build by dropping the vmxnet3 debug printf fixes
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>

Applied this version; thanks for the fast turnaround.

-- PMM