[PATCH] um: vector: Possible underflow and races fixes
anton.ivanov at cambridgegreys.com
anton.ivanov at cambridgegreys.com
Wed Oct 21 06:05:15 PDT 2026
From: Anton Ivanov <anton.ivanov at cambridgegreys.com>
Michael S. Tsirkin while working on net virtio checksum hardening
discovered a number of issues in the uml vector driver. This patch
addresses these issues.
1. Use of header_size, not rx_header_size in some of the runt/underflow
checks. IPV6 raw socket API for protocols like GRE or L2TPV2 is asymmetric -
the header format and size in recv and send differ. All rx checks must use
the rx specific size.
2. Vector reuses rx and tx queue locks for stats. These queues are not
always allocated as some transports do not use them and they can be
disabled on the kernel command line. Resetting or accessing ethtool stats
in this case could lead to a null pointer dereference.
3. virtio_net_header processing was not up to date with all the GSO
additions and changes and was skipping necessary processing.
4. Missing barrier in vector_send().
5. Incorrect timer destruction in net_close()
Signed-off-by: Anton Ivanov <anton.ivanov at cambridgegreys.com>
---
arch/um/drivers/vector_kern.c | 65 +++++++++++++++++++----------
arch/um/drivers/vector_kern.h | 2 -
arch/um/drivers/vector_transports.c | 3 --
3 files changed, 43 insertions(+), 27 deletions(-)
diff --git a/arch/um/drivers/vector_kern.c b/arch/um/drivers/vector_kern.c
index 8a70b3a62553..b8e3cc21d7da 100644
--- a/arch/um/drivers/vector_kern.c
+++ b/arch/um/drivers/vector_kern.c
@@ -108,22 +108,26 @@ static void vector_reset_stats(struct vector_private *vp)
/* We reuse the existing queue locks for stats */
/* RX stats are modified with RX head_lock held
- * in vector_poll.
+ * in vector_poll when using vector RX.
*/
- spin_lock(&vp->rx_queue->head_lock);
+ if (vp->rx_queue)
+ spin_lock(&vp->rx_queue->head_lock);
+
vp->estats.rx_queue_max = 0;
vp->estats.rx_queue_running_average = 0;
vp->estats.rx_encaps_errors = 0;
vp->estats.sg_ok = 0;
vp->estats.sg_linearized = 0;
- spin_unlock(&vp->rx_queue->head_lock);
+ if (vp->rx_queue)
+ spin_unlock(&vp->rx_queue->head_lock);
/* TX stats are modified with TX head_lock held
- * in vector_send.
+ * in vector_send when doing vector TX.
*/
- spin_lock(&vp->tx_queue->head_lock);
+ if (vp->tx_queue)
+ spin_lock(&vp->tx_queue->head_lock);
vp->estats.tx_timeout_count = 0;
vp->estats.tx_restart_queue = 0;
vp->estats.tx_kicks = 0;
@@ -131,7 +135,9 @@ static void vector_reset_stats(struct vector_private *vp)
vp->estats.tx_flow_control_xoff = 0;
vp->estats.tx_queue_max = 0;
vp->estats.tx_queue_running_average = 0;
- spin_unlock(&vp->tx_queue->head_lock);
+ if (vp->tx_queue)
+ spin_unlock(&vp->tx_queue->head_lock);
+
}
static int get_mtu(struct arglist *def)
@@ -226,6 +232,11 @@ static int get_transport_options(struct arglist *def)
}
}
+ if (get_depth(def) <= 0) {
+ vec_rx = 0;
+ vec_tx = 0;
+ }
+
if (get_bpf_flash(def))
result = VECTOR_BPF_FLASH;
@@ -406,6 +417,7 @@ static int vector_send(struct vector_queue *qi)
/* update queue_depth to current value */
while (atomic_read(&qi->queue_depth) > 0) {
/* Calculate the start of the vector */
+ smp_rmb();
send_len = atomic_read(&qi->queue_depth);
send_from = qi->mmsg_vector;
send_from += qi->head;
@@ -864,18 +876,17 @@ static int vector_legacy_rx(struct vector_private *vp)
}
if (skb != NULL) {
- if (pkt_len > vp->header_size) {
- if (vp->header_size > 0) {
+ if (pkt_len > vp->rx_header_size) {
+ if (vp->rx_header_size > 0) {
header_check = vp->verify_header(
vp->header_rxbuffer, skb, vp);
if (header_check < 0) {
dev_kfree_skb_irq(skb);
vp->dev->stats.rx_dropped++;
- vp->estats.rx_encaps_errors++;
+ dev_kfree_skb_irq(skb);
return 0;
}
if (header_check > 0) {
- vp->estats.rx_csum_offload_good++;
skb->ip_summed = CHECKSUM_UNNECESSARY;
}
}
@@ -980,7 +991,7 @@ static int vector_mmsg_rx(struct vector_private *vp, int budget)
for (i = 0; i < packet_count; i++) {
skb = (*skbuff_vector);
- if (mmsg_vector->msg_len > vp->header_size) {
+ if (mmsg_vector->msg_len > vp->rx_header_size) {
if (vp->header_size > 0) {
header_check = vp->verify_header(
mmsg_vector->msg_hdr.msg_iov->iov_base,
@@ -1111,7 +1122,8 @@ static int vector_net_close(struct net_device *dev)
struct vector_private *vp = netdev_priv(dev);
netif_stop_queue(dev);
- timer_delete(&vp->tl);
+ if (vp->options & VECTOR_TX)
+ timer_shutdown_sync(&vp->tl);
vp->opened = false;
@@ -1166,15 +1178,15 @@ static int vector_poll(struct napi_struct *napi, int budget)
if ((vp->options & VECTOR_TX) != 0)
tx_enqueued = (vector_send(vp->tx_queue) > 0);
- spin_lock(&vp->rx_queue->head_lock);
- if ((vp->options & VECTOR_RX) > 0)
+ if ((vp->options & VECTOR_RX) > 0) {
+ spin_lock(&vp->rx_queue->head_lock);
err = vector_mmsg_rx(vp, budget);
- else {
+ spin_unlock(&vp->rx_queue->head_lock);
+ } else {
err = vector_legacy_rx(vp);
if (err > 0)
err = 1;
}
- spin_unlock(&vp->rx_queue->head_lock);
if (err > 0)
work_done += err;
@@ -1242,9 +1254,13 @@ static int vector_net_open(struct net_device *dev)
if (vp->header_txbuffer == NULL)
goto out_close;
}
+ if ((vp->options & (VECTOR_TX | VECTOR_RX)) &&
+ (get_depth(vp->parsed) > (NAPI_POLL_WEIGHT << 2)))
+ netif_napi_add_weight(vp->dev, &vp->napi, vector_poll,
+ get_depth(vp->parsed));
+ else
+ netif_napi_add_weight(vp->dev, &vp->napi, vector_poll, NAPI_POLL_WEIGHT);
- netif_napi_add_weight(vp->dev, &vp->napi, vector_poll,
- get_depth(vp->parsed));
napi_enable(&vp->napi);
/* READ IRQ */
@@ -1302,6 +1318,7 @@ static int vector_net_open(struct net_device *dev)
if ((vp->options & VECTOR_TX) != 0)
add_timer(&vp->tl);
+
return 0;
out_close:
vector_net_close(dev);
@@ -1469,11 +1486,15 @@ static void vector_get_ethtool_stats(struct net_device *dev,
* to date.
*/
- spin_lock(&vp->tx_queue->head_lock);
- spin_lock(&vp->rx_queue->head_lock);
+ if (vp->tx_queue)
+ spin_lock(&vp->tx_queue->head_lock);
+ if (vp->rx_queue)
+ spin_lock(&vp->rx_queue->head_lock);
memcpy(tmp_stats, &vp->estats, sizeof(struct vector_estats));
- spin_unlock(&vp->rx_queue->head_lock);
- spin_unlock(&vp->tx_queue->head_lock);
+ if (vp->rx_queue)
+ spin_unlock(&vp->rx_queue->head_lock);
+ if (vp->tx_queue)
+ spin_unlock(&vp->tx_queue->head_lock);
}
static int vector_get_coalesce(struct net_device *netdev,
diff --git a/arch/um/drivers/vector_kern.h b/arch/um/drivers/vector_kern.h
index 417834793658..5f0ceb80779d 100644
--- a/arch/um/drivers/vector_kern.h
+++ b/arch/um/drivers/vector_kern.h
@@ -116,8 +116,6 @@ struct vector_private {
int (*verify_header)(uint8_t *header,
struct sk_buff *skb, struct vector_private *vp);
- spinlock_t stats_lock;
-
bool rexmit_scheduled;
bool opened;
bool in_write_poll;
diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c
index e1fb2a76fdff..786f6badc52d 100644
--- a/arch/um/drivers/vector_transports.c
+++ b/arch/um/drivers/vector_transports.c
@@ -206,9 +206,6 @@ static int raw_verify_header(
GSO_ERROR
);
}
- if ((vheader->flags & VIRTIO_NET_HDR_F_DATA_VALID) > 0)
- return 1;
-
skb_set_network_header(skb, ETH_HLEN);
virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian());
return 0;
--
2.47.3
More information about the linux-um
mailing list