From 5930e5ccf38158b090f38554994dab604f01f017 Mon Sep 17 00:00:00 2001
From: Akihiko Odaki <akihiko.odaki@daynix.com>
Date: Fri, 22 Nov 2024 14:03:07 +0900
Subject: [PATCH 1/6] net: checksum: Convert data to void *
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Convert the data parameter of net_checksum_calculate() to void * to
save unnecessary casts for callers.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
include/net/checksum.h | 2 +-
net/checksum.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/net/checksum.h b/include/net/checksum.h
index 7dec37e56c..188e4cca0b 100644
--- a/include/net/checksum.h
+++ b/include/net/checksum.h
@@ -30,7 +30,7 @@ uint32_t net_checksum_add_cont(int len, uint8_t *buf, int seq);
uint16_t net_checksum_finish(uint32_t sum);
uint16_t net_checksum_tcpudp(uint16_t length, uint16_t proto,
uint8_t *addrs, uint8_t *buf);
-void net_checksum_calculate(uint8_t *data, int length, int csum_flag);
+void net_checksum_calculate(void *data, int length, int csum_flag);
static inline uint32_t
net_checksum_add(int len, uint8_t *buf)
diff --git a/net/checksum.c b/net/checksum.c
index 1a957e4c0b..537457d89d 100644
--- a/net/checksum.c
+++ b/net/checksum.c
@@ -57,7 +57,7 @@ uint16_t net_checksum_tcpudp(uint16_t length, uint16_t proto,
return net_checksum_finish(sum);
}
-void net_checksum_calculate(uint8_t *data, int length, int csum_flag)
+void net_checksum_calculate(void *data, int length, int csum_flag)
{
int mac_hdr_len, ip_len;
struct ip_header *ip;
@@ -101,7 +101,7 @@ void net_checksum_calculate(uint8_t *data, int length, int csum_flag)
return;
}
- ip = (struct ip_header *)(data + mac_hdr_len);
+ ip = (struct ip_header *)((uint8_t *)data + mac_hdr_len);
if (IP_HEADER_VERSION(ip) != IP_HEADER_VERSION_4) {
return; /* not IPv4 */
From a8575f7fb2f213e6690b23160b04271d47fdfaa8 Mon Sep 17 00:00:00 2001
From: Akihiko Odaki <akihiko.odaki@daynix.com>
Date: Fri, 22 Nov 2024 14:03:08 +0900
Subject: [PATCH 2/6] virtio-net: Fix size check in dhclient workaround
work_around_broken_dhclient() accesses IP and UDP headers to detect
relevant packets and to calculate checksums, but it didn't check if
the packet has size sufficient to accommodate them, causing out-of-bound
access hazards. Fix this by correcting the size requirement.
Fixes: 1d41b0c1ec66 ("Work around dhclient brokenness")
Cc: qemu-stable@nongnu.org
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
hw/net/virtio-net.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 75b4a28fb3..a2a8d6b07b 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1697,8 +1697,11 @@ static void virtio_net_hdr_swap(VirtIODevice *vdev, struct virtio_net_hdr *hdr)
static void work_around_broken_dhclient(struct virtio_net_hdr *hdr,
uint8_t *buf, size_t size)
{
+ size_t csum_size = ETH_HLEN + sizeof(struct ip_header) +
+ sizeof(struct udp_header);
+
if ((hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) && /* missing csum */
- (size > 27 && size < 1500) && /* normal sized MTU */
+ (size >= csum_size && size < 1500) && /* normal sized MTU */
(buf[12] == 0x08 && buf[13] == 0x00) && /* ethertype == IPv4 */
(buf[23] == 17) && /* ip.protocol == UDP */
(buf[34] == 0 && buf[35] == 67)) { /* udp.srcport == bootps */
From 162bdb8113d32e2efb3c6967e3828d4f0c4e14e0 Mon Sep 17 00:00:00 2001
From: Akihiko Odaki <akihiko.odaki@daynix.com>
Date: Fri, 22 Nov 2024 14:03:09 +0900
Subject: [PATCH 3/6] virtio-net: Do not check for the queue before RSS
virtio_net_can_receive() checks if the queue is ready, but RSS will
change the queue to use so, strictly speaking, we may still be able to
receive the packet even if the queue initially provided is not ready.
Perform RSS before virtio_net_can_receive() to cover such a case.
Fixes: 4474e37a5b3a ("virtio-net: implement RX RSS processing")
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
hw/net/virtio-net.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index a2a8d6b07b..d4aaf362b7 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1911,10 +1911,6 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
size_t offset, i, guest_offset, j;
ssize_t err;
- if (!virtio_net_can_receive(nc)) {
- return -1;
- }
-
if (!no_rss && n->rss_data.enabled && n->rss_data.enabled_software_rss) {
int index = virtio_net_process_rss(nc, buf, size, &extra_hdr);
if (index >= 0) {
@@ -1924,6 +1920,10 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
}
}
+ if (!virtio_net_can_receive(nc)) {
+ return -1;
+ }
+
/* hdr_len refers to the header we supply to the guest */
if (!virtio_net_has_buffers(q, size + n->guest_hdr_len - n->host_hdr_len)) {
return 0;
From 1981fa9d7da7a763b38ca2f583b9ec3a335cf661 Mon Sep 17 00:00:00 2001
From: Akihiko Odaki <akihiko.odaki@daynix.com>
Date: Fri, 22 Nov 2024 14:03:10 +0900
Subject: [PATCH 4/6] virtio-net: Fix hash reporting when the queue changes
virtio_net_process_rss() fills the values used for hash reporting, but
the values used to be thrown away with a recursive function call if
the queue changes after RSS. Avoid the function call to keep the values.
Fixes: a4c960eedcd2 ("virtio-net: Do not write hashes to peer buffer")
Buglink: https://issues.redhat.com/browse/RHEL-59572
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
hw/net/virtio-net.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index d4aaf362b7..b544d2e91a 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1898,10 +1898,10 @@ static int virtio_net_process_rss(NetClientState *nc, const uint8_t *buf,
}
static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
- size_t size, bool no_rss)
+ size_t size)
{
VirtIONet *n = qemu_get_nic_opaque(nc);
- VirtIONetQueue *q = virtio_net_get_subqueue(nc);
+ VirtIONetQueue *q;
VirtIODevice *vdev = VIRTIO_DEVICE(n);
VirtQueueElement *elems[VIRTQUEUE_MAX_SIZE];
size_t lens[VIRTQUEUE_MAX_SIZE];
@@ -1911,12 +1911,10 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
size_t offset, i, guest_offset, j;
ssize_t err;
- if (!no_rss && n->rss_data.enabled && n->rss_data.enabled_software_rss) {
+ if (n->rss_data.enabled && n->rss_data.enabled_software_rss) {
int index = virtio_net_process_rss(nc, buf, size, &extra_hdr);
if (index >= 0) {
- NetClientState *nc2 =
- qemu_get_subqueue(n->nic, index % n->curr_queue_pairs);
- return virtio_net_receive_rcu(nc2, buf, size, true);
+ nc = qemu_get_subqueue(n->nic, index % n->curr_queue_pairs);
}
}
@@ -1924,6 +1922,8 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
return -1;
}
+ q = virtio_net_get_subqueue(nc);
+
/* hdr_len refers to the header we supply to the guest */
if (!virtio_net_has_buffers(q, size + n->guest_hdr_len - n->host_hdr_len)) {
return 0;
@@ -2049,7 +2049,7 @@ static ssize_t virtio_net_do_receive(NetClientState *nc, const uint8_t *buf,
{
RCU_READ_LOCK_GUARD();
- return virtio_net_receive_rcu(nc, buf, size, false);
+ return virtio_net_receive_rcu(nc, buf, size);
}
/*
From 17437418c4ed0b779f7e9747fbffb08e139aa0ae Mon Sep 17 00:00:00 2001
From: Akihiko Odaki <akihiko.odaki@daynix.com>
Date: Fri, 22 Nov 2024 14:03:11 +0900
Subject: [PATCH 5/6] virtio-net: Initialize hash reporting values
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The specification says hash_report should be set to
VIRTIO_NET_HASH_REPORT_NONE if VIRTIO_NET_F_HASH_REPORT is negotiated
but not configured with VIRTIO_NET_CTRL_MQ_RSS_CONFIG. However,
virtio_net_receive_rcu() instead wrote out the content of the extra_hdr
variable, which is not uninitialized in such a case.
Fix this by zeroing the extra_hdr.
Fixes: e22f0603fb2f ("virtio-net: reference implementation of hash report")
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
hw/net/virtio-net.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index b544d2e91a..44ea72b50e 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1911,6 +1911,8 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
size_t offset, i, guest_offset, j;
ssize_t err;
+ memset(&extra_hdr, 0, sizeof(extra_hdr));
+
if (n->rss_data.enabled && n->rss_data.enabled_software_rss) {
int index = virtio_net_process_rss(nc, buf, size, &extra_hdr);
if (index >= 0) {
From 7987d2be5a8bc3a502f89ba8cf3ac3e09f64d1ce Mon Sep 17 00:00:00 2001
From: Akihiko Odaki <akihiko.odaki@daynix.com>
Date: Fri, 22 Nov 2024 14:03:12 +0900
Subject: [PATCH 6/6] virtio-net: Copy received header to buffer
receive_header() used to cast the const qualifier of the pointer to the
received packet away to modify the header. Avoid this by copying the
received header to buffer.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
hw/net/virtio-net.c | 85 ++++++++++++++++++++++++---------------------
1 file changed, 46 insertions(+), 39 deletions(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 44ea72b50e..bc0f31b666 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1695,41 +1695,44 @@ static void virtio_net_hdr_swap(VirtIODevice *vdev, struct virtio_net_hdr *hdr)
* cache.
*/
static void work_around_broken_dhclient(struct virtio_net_hdr *hdr,
- uint8_t *buf, size_t size)
+ size_t *hdr_len, const uint8_t *buf,
+ size_t buf_size, size_t *buf_offset)
{
size_t csum_size = ETH_HLEN + sizeof(struct ip_header) +
sizeof(struct udp_header);
+ buf += *buf_offset;
+ buf_size -= *buf_offset;
+
if ((hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) && /* missing csum */
- (size >= csum_size && size < 1500) && /* normal sized MTU */
+ (buf_size >= csum_size && buf_size < 1500) && /* normal sized MTU */
(buf[12] == 0x08 && buf[13] == 0x00) && /* ethertype == IPv4 */
(buf[23] == 17) && /* ip.protocol == UDP */
(buf[34] == 0 && buf[35] == 67)) { /* udp.srcport == bootps */
- net_checksum_calculate(buf, size, CSUM_UDP);
+ memcpy((uint8_t *)hdr + *hdr_len, buf, csum_size);
+ net_checksum_calculate((uint8_t *)hdr + *hdr_len, csum_size, CSUM_UDP);
hdr->flags &= ~VIRTIO_NET_HDR_F_NEEDS_CSUM;
+ *hdr_len += csum_size;
+ *buf_offset += csum_size;
}
}
-static void receive_header(VirtIONet *n, const struct iovec *iov, int iov_cnt,
- const void *buf, size_t size)
+static size_t receive_header(VirtIONet *n, struct virtio_net_hdr *hdr,
+ const void *buf, size_t buf_size,
+ size_t *buf_offset)
{
- if (n->has_vnet_hdr) {
- /* FIXME this cast is evil */
- void *wbuf = (void *)buf;
- work_around_broken_dhclient(wbuf, wbuf + n->host_hdr_len,
- size - n->host_hdr_len);
+ size_t hdr_len = n->guest_hdr_len;
- if (n->needs_vnet_hdr_swap) {
- virtio_net_hdr_swap(VIRTIO_DEVICE(n), wbuf);
- }
- iov_from_buf(iov, iov_cnt, 0, buf, sizeof(struct virtio_net_hdr));
- } else {
- struct virtio_net_hdr hdr = {
- .flags = 0,
- .gso_type = VIRTIO_NET_HDR_GSO_NONE
- };
- iov_from_buf(iov, iov_cnt, 0, &hdr, sizeof hdr);
+ memcpy(hdr, buf, sizeof(struct virtio_net_hdr));
+
+ *buf_offset = n->host_hdr_len;
+ work_around_broken_dhclient(hdr, &hdr_len, buf, buf_size, buf_offset);
+
+ if (n->needs_vnet_hdr_swap) {
+ virtio_net_hdr_swap(VIRTIO_DEVICE(n), hdr);
}
+
+ return hdr_len;
}
static int receive_filter(VirtIONet *n, const uint8_t *buf, int size)
@@ -1897,6 +1900,13 @@ static int virtio_net_process_rss(NetClientState *nc, const uint8_t *buf,
return (index == new_index) ? -1 : new_index;
}
+typedef struct Header {
+ struct virtio_net_hdr_v1_hash virtio_net;
+ struct eth_header eth;
+ struct ip_header ip;
+ struct udp_header udp;
+} Header;
+
static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
size_t size)
{
@@ -1906,15 +1916,15 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
VirtQueueElement *elems[VIRTQUEUE_MAX_SIZE];
size_t lens[VIRTQUEUE_MAX_SIZE];
struct iovec mhdr_sg[VIRTQUEUE_MAX_SIZE];
- struct virtio_net_hdr_v1_hash extra_hdr;
+ Header hdr;
unsigned mhdr_cnt = 0;
size_t offset, i, guest_offset, j;
ssize_t err;
- memset(&extra_hdr, 0, sizeof(extra_hdr));
+ memset(&hdr.virtio_net, 0, sizeof(hdr.virtio_net));
if (n->rss_data.enabled && n->rss_data.enabled_software_rss) {
- int index = virtio_net_process_rss(nc, buf, size, &extra_hdr);
+ int index = virtio_net_process_rss(nc, buf, size, &hdr.virtio_net);
if (index >= 0) {
nc = qemu_get_subqueue(n->nic, index % n->curr_queue_pairs);
}
@@ -1979,21 +1989,18 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
if (n->mergeable_rx_bufs) {
mhdr_cnt = iov_copy(mhdr_sg, ARRAY_SIZE(mhdr_sg),
sg, elem->in_num,
- offsetof(typeof(extra_hdr), hdr.num_buffers),
- sizeof(extra_hdr.hdr.num_buffers));
+ offsetof(typeof(hdr),
+ virtio_net.hdr.num_buffers),
+ sizeof(hdr.virtio_net.hdr.num_buffers));
}
- receive_header(n, sg, elem->in_num, buf, size);
- if (n->rss_data.populate_hash) {
- offset = offsetof(typeof(extra_hdr), hash_value);
- iov_from_buf(sg, elem->in_num, offset,
- (char *)&extra_hdr + offset,
- sizeof(extra_hdr.hash_value) +
- sizeof(extra_hdr.hash_report));
- }
- offset = n->host_hdr_len;
- total += n->guest_hdr_len;
- guest_offset = n->guest_hdr_len;
+ guest_offset = n->has_vnet_hdr ?
+ receive_header(n, (struct virtio_net_hdr *)&hdr,
+ buf, size, &offset) :
+ n->guest_hdr_len;
+
+ iov_from_buf(sg, elem->in_num, 0, &hdr, guest_offset);
+ total += guest_offset;
} else {
guest_offset = 0;
}
@@ -2019,11 +2026,11 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
}
if (mhdr_cnt) {
- virtio_stw_p(vdev, &extra_hdr.hdr.num_buffers, i);
+ virtio_stw_p(vdev, &hdr.virtio_net.hdr.num_buffers, i);
iov_from_buf(mhdr_sg, mhdr_cnt,
0,
- &extra_hdr.hdr.num_buffers,
- sizeof extra_hdr.hdr.num_buffers);
+ &hdr.virtio_net.hdr.num_buffers,
+ sizeof hdr.virtio_net.hdr.num_buffers);
}
for (j = 0; j < i; j++) {