From 7da2d99fb9fbf30104125c061caaff330e362d74 Mon Sep 17 00:00:00 2001 From: liujunjie Date: Mon, 17 Sep 2018 21:48:45 +0800 Subject: clean up callback when del virtqueue Before, we did not clear callback like handle_output when delete the virtqueue which may result be segmentfault. The scene is as follows: 1. Start a vm with multiqueue vhost-net, 2. then we write VIRTIO_PCI_GUEST_FEATURES in PCI configuration to triger multiqueue disable in this vm which will delete the virtqueue. In this step, the tx_bh is deleted but the callback virtio_net_handle_tx_bh still exist. 3. Finally, we write VIRTIO_PCI_QUEUE_NOTIFY in PCI configuration to notify the deleted virtqueue. In this way, virtio_net_handle_tx_bh will be called and qemu will be crashed. Although the way described above is uncommon, we had better reinforce it. CC: qemu-stable@nongnu.org Signed-off-by: liujunjie Signed-off-by: Jason Wang --- hw/virtio/virtio.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'hw') diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 4e61944..4136d23 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -1611,6 +1611,8 @@ void virtio_del_queue(VirtIODevice *vdev, int n) vdev->vq[n].vring.num = 0; vdev->vq[n].vring.num_default = 0; + vdev->vq[n].handle_output = NULL; + vdev->vq[n].handle_aio_output = NULL; } static void virtio_set_isr(VirtIODevice *vdev, int value) -- cgit v1.1 From fdc89e90fac40c5ca2686733df17b6423fb8d8fb Mon Sep 17 00:00:00 2001 From: Jason Wang Date: Wed, 30 May 2018 13:08:15 +0800 Subject: ne2000: fix possible out of bound access in ne2000_receive In ne2000_receive(), we try to assign size_ to size which converts from size_t to integer. This will cause troubles when size_ is greater INT_MAX, this will lead a negative value in size and it can then pass the check of size < MIN_BUF_SIZE which may lead out of bound access of for both buf and buf1. Fixing by converting the type of size to size_t. CC: qemu-stable@nongnu.org Reported-by: Daniel Shapira Reviewed-by: Michael S. Tsirkin Signed-off-by: Jason Wang --- hw/net/ne2000.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'hw') diff --git a/hw/net/ne2000.c b/hw/net/ne2000.c index 07d79e3..869518e 100644 --- a/hw/net/ne2000.c +++ b/hw/net/ne2000.c @@ -174,7 +174,7 @@ static int ne2000_buffer_full(NE2000State *s) ssize_t ne2000_receive(NetClientState *nc, const uint8_t *buf, size_t size_) { NE2000State *s = qemu_get_nic_opaque(nc); - int size = size_; + size_t size = size_; uint8_t *p; unsigned int total_len, next, avail, len, index, mcast_idx; uint8_t buf1[60]; @@ -182,7 +182,7 @@ ssize_t ne2000_receive(NetClientState *nc, const uint8_t *buf, size_t size_) { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }; #if defined(DEBUG_NE2000) - printf("NE2000: received len=%d\n", size); + printf("NE2000: received len=%zu\n", size); #endif if (s->cmd & E8390_STOP || ne2000_buffer_full(s)) -- cgit v1.1 From 1a326646fef38782e5542280040ec3ea23e4a730 Mon Sep 17 00:00:00 2001 From: Jason Wang Date: Wed, 30 May 2018 13:07:43 +0800 Subject: rtl8139: fix possible out of bound access In rtl8139_do_receive(), we try to assign size_ to size which converts from size_t to integer. This will cause troubles when size_ is greater INT_MAX, this will lead a negative value in size and it can then pass the check of size < MIN_BUF_SIZE which may lead out of bound access of for both buf and buf1. Fixing by converting the type of size to size_t. CC: qemu-stable@nongnu.org Reported-by: Daniel Shapira Reviewed-by: Michael S. Tsirkin Signed-off-by: Jason Wang --- hw/net/rtl8139.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'hw') diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c index 46daa16..2342a09 100644 --- a/hw/net/rtl8139.c +++ b/hw/net/rtl8139.c @@ -817,7 +817,7 @@ static ssize_t rtl8139_do_receive(NetClientState *nc, const uint8_t *buf, size_t RTL8139State *s = qemu_get_nic_opaque(nc); PCIDevice *d = PCI_DEVICE(s); /* size is the length of the buffer passed to the driver */ - int size = size_; + size_t size = size_; const uint8_t *dot1q_buf = NULL; uint32_t packet_header = 0; @@ -826,7 +826,7 @@ static ssize_t rtl8139_do_receive(NetClientState *nc, const uint8_t *buf, size_t static const uint8_t broadcast_macaddr[6] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }; - DPRINTF(">>> received len=%d\n", size); + DPRINTF(">>> received len=%zu\n", size); /* test if board clock is stopped */ if (!s->clock_enabled) @@ -1035,7 +1035,7 @@ static ssize_t rtl8139_do_receive(NetClientState *nc, const uint8_t *buf, size_t if (size+4 > rx_space) { - DPRINTF("C+ Rx mode : descriptor %d size %d received %d + 4\n", + DPRINTF("C+ Rx mode : descriptor %d size %d received %zu + 4\n", descriptor, rx_space, size); s->IntrStatus |= RxOverflow; @@ -1148,7 +1148,7 @@ static ssize_t rtl8139_do_receive(NetClientState *nc, const uint8_t *buf, size_t if (avail != 0 && RX_ALIGN(size + 8) >= avail) { DPRINTF("rx overflow: rx buffer length %d head 0x%04x " - "read 0x%04x === available 0x%04x need 0x%04x\n", + "read 0x%04x === available 0x%04x need 0x%04zx\n", s->RxBufferSize, s->RxBufAddr, s->RxBufPtr, avail, size + 8); s->IntrStatus |= RxOverflow; -- cgit v1.1 From b1d80d12c5f7ff081bb80ab4f4241d4248691192 Mon Sep 17 00:00:00 2001 From: Jason Wang Date: Wed, 30 May 2018 12:11:30 +0800 Subject: pcnet: fix possible buffer overflow In pcnet_receive(), we try to assign size_ to size which converts from size_t to integer. This will cause troubles when size_ is greater INT_MAX, this will lead a negative value in size and it can then pass the check of size < MIN_BUF_SIZE which may lead out of bound access for both buf and buf1. Fixing by converting the type of size to size_t. CC: qemu-stable@nongnu.org Reported-by: Daniel Shapira Reviewed-by: Michael S. Tsirkin Signed-off-by: Jason Wang --- hw/net/pcnet.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'hw') diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c index 0c44554..d9ba04b 100644 --- a/hw/net/pcnet.c +++ b/hw/net/pcnet.c @@ -988,14 +988,14 @@ ssize_t pcnet_receive(NetClientState *nc, const uint8_t *buf, size_t size_) uint8_t buf1[60]; int remaining; int crc_err = 0; - int size = size_; + size_t size = size_; if (CSR_DRX(s) || CSR_STOP(s) || CSR_SPND(s) || !size || (CSR_LOOP(s) && !s->looptest)) { return -1; } #ifdef PCNET_DEBUG - printf("pcnet_receive size=%d\n", size); + printf("pcnet_receive size=%zu\n", size); #endif /* if too small buffer, then expand it */ -- cgit v1.1 From 1001cf45a7ca22d990e19a81029319ca763cad4f Mon Sep 17 00:00:00 2001 From: Jason Wang Date: Tue, 16 Oct 2018 17:40:45 +0800 Subject: e1000: indicate dropped packets in HW counters The e1000 emulation silently discards RX packets if there's insufficient space in the ring buffer. This leads to errors on higher-level protocols in the guest, with no indication about the error cause. This patch increments the "Missed Packets Count" (MPC) and "Receive No Buffers Count" (RNBC) HW counters in this case. As the emulation has no FIFO for buffering packets that can't immediately be pushed to the guest, these two registers are practically equivalent (see 10.2.7.4, 10.2.7.33 in https://www.intel.com/content/www/us/en/embedded/products/networking/82574l-gbe-controller-datasheet.html). On a Linux guest, the register content will be reflected in the "rx_missed_errors" and "rx_no_buffer_count" stats from "ethtool -S", and in the "missed" stat from "ip -s -s link show", giving at least some hint about the error cause inside the guest. If the cause is known, problems like this can often be avoided easily, by increasing the number of RX descriptors in the guest e1000 driver (e.g under Linux, "e1000.RxDescriptors=1024"). The patch also adds a qemu trace message for this condition. Signed-off-by: Martin Wilck Signed-off-by: Jason Wang --- hw/net/e1000.c | 16 +++++++++++++--- hw/net/trace-events | 3 +++ 2 files changed, 16 insertions(+), 3 deletions(-) (limited to 'hw') diff --git a/hw/net/e1000.c b/hw/net/e1000.c index 13a9494..5e144cb 100644 --- a/hw/net/e1000.c +++ b/hw/net/e1000.c @@ -36,6 +36,7 @@ #include "qemu/range.h" #include "e1000x_common.h" +#include "trace.h" static const uint8_t bcast[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff}; @@ -847,6 +848,15 @@ static uint64_t rx_desc_base(E1000State *s) return (bah << 32) + bal; } +static void +e1000_receiver_overrun(E1000State *s, size_t size) +{ + trace_e1000_receiver_overrun(size, s->mac_reg[RDH], s->mac_reg[RDT]); + e1000x_inc_reg_if_not_full(s->mac_reg, RNBC); + e1000x_inc_reg_if_not_full(s->mac_reg, MPC); + set_ics(s, 0, E1000_ICS_RXO); +} + static ssize_t e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt) { @@ -916,8 +926,8 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt) desc_offset = 0; total_size = size + e1000x_fcs_len(s->mac_reg); if (!e1000_has_rxbufs(s, total_size)) { - set_ics(s, 0, E1000_ICS_RXO); - return -1; + e1000_receiver_overrun(s, total_size); + return -1; } do { desc_size = total_size - desc_offset; @@ -969,7 +979,7 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt) rdh_start >= s->mac_reg[RDLEN] / sizeof(desc)) { DBGOUT(RXERR, "RDH wraparound @%x, RDT %x, RDLEN %x\n", rdh_start, s->mac_reg[RDT], s->mac_reg[RDLEN]); - set_ics(s, 0, E1000_ICS_RXO); + e1000_receiver_overrun(s, total_size); return -1; } } while (desc_offset < total_size); diff --git a/hw/net/trace-events b/hw/net/trace-events index c1dea4b..9d49f62 100644 --- a/hw/net/trace-events +++ b/hw/net/trace-events @@ -98,6 +98,9 @@ net_rx_pkt_rss_ip6_ex(void) "Calculating IPv6/EX RSS hash" net_rx_pkt_rss_hash(size_t rss_length, uint32_t rss_hash) "RSS hash for %zu bytes: 0x%X" net_rx_pkt_rss_add_chunk(void* ptr, size_t size, size_t input_offset) "Add RSS chunk %p, %zu bytes, RSS input offset %zu bytes" +# hw/net/e1000.c +e1000_receiver_overrun(size_t s, uint32_t rdh, uint32_t rdt) "Receiver overrun: dropped packet of %zu bytes, RDH=%u, RDT=%u" + # hw/net/e1000x_common.c e1000x_rx_can_recv_disabled(bool link_up, bool rx_enabled, bool pci_master) "link_up: %d, rx_enabled %d, pci_master %d" e1000x_vlan_is_vlan_pkt(bool is_vlan_pkt, uint16_t eth_proto, uint16_t vet) "Is VLAN packet: %d, ETH proto: 0x%X, VET: 0x%X" -- cgit v1.1