net: vwifi: fix missing tx statistics in virtio mode#95
Conversation
In virtio mode, the vwifi_ndo_start_xmit() directly returns the result of vwifi_virtio_tx() without updating the transmission statistics. This patch adds the proper statistics accounting (tx_packets, tx_bytes, and tx_dropped) directly inside vwifi_virtio_tx(). Signed-off-by: liting <ga13234624@gmail.com>
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="vwifi.c">
<violation number="1" location="vwifi.c:3179">
P1: This path incorrectly counts dropped packets when `virtqueue_kick()` suppresses a notification. In the Linux virtio API, `virtqueue_kick()` returning `false` means the notification was not needed (suppressed), not that transmission failed. After `virtqueue_add_outbuf()` succeeds, the `skb` is owned by the virtqueue and the completion handler (`vwifi_virtio_tx_done`) will free it. Jumping to `out_free` here will both miscount `tx_dropped` and free an skb still owned by the virtqueue, risking a double-free or UAF when TX completion later frees it.
The established kernel pattern — as fixed in virtio_net.c — is to call `virtqueue_kick()` without checking its return value, because the buffers are already committed to the virtqueue regardless of whether a notification is sent.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| return NETDEV_TX_OK; | ||
|
|
||
| out_free: | ||
| vif->stats.tx_dropped++; |
There was a problem hiding this comment.
P1: This path incorrectly counts dropped packets when virtqueue_kick() suppresses a notification. In the Linux virtio API, virtqueue_kick() returning false means the notification was not needed (suppressed), not that transmission failed. After virtqueue_add_outbuf() succeeds, the skb is owned by the virtqueue and the completion handler (vwifi_virtio_tx_done) will free it. Jumping to out_free here will both miscount tx_dropped and free an skb still owned by the virtqueue, risking a double-free or UAF when TX completion later frees it.
The established kernel pattern — as fixed in virtio_net.c — is to call virtqueue_kick() without checking its return value, because the buffers are already committed to the virtqueue regardless of whether a notification is sent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At vwifi.c, line 3179:
<comment>This path incorrectly counts dropped packets when `virtqueue_kick()` suppresses a notification. In the Linux virtio API, `virtqueue_kick()` returning `false` means the notification was not needed (suppressed), not that transmission failed. After `virtqueue_add_outbuf()` succeeds, the `skb` is owned by the virtqueue and the completion handler (`vwifi_virtio_tx_done`) will free it. Jumping to `out_free` here will both miscount `tx_dropped` and free an skb still owned by the virtqueue, risking a double-free or UAF when TX completion later frees it.
The established kernel pattern — as fixed in virtio_net.c — is to call `virtqueue_kick()` without checking its return value, because the buffers are already committed to the virtqueue regardless of whether a notification is sent.</comment>
<file context>
@@ -3168,11 +3169,14 @@ static netdev_tx_t vwifi_virtio_tx(struct vwifi_vif *vif, struct sk_buff *skb)
return NETDEV_TX_OK;
out_free:
+ vif->stats.tx_dropped++;
spin_unlock_irqrestore(&vwifi_virtio_lock, flags);
dev_kfree_skb(skb);
</file context>
|
@ga13234624 Great work! It's always nice to see these small consistency fixes that improve correctness. Thanks for taking care of it. |
In virtio mode, the vwifi_ndo_start_xmit() directly returns the result of vwifi_virtio_tx() without updating the transmission statistics.
This patch adds the proper statistics accounting (tx_packets, tx_bytes, and tx_dropped) directly inside vwifi_virtio_tx().
Summary by cubic
Fixes missing TX statistics in virtio mode by updating counters inside vwifi_virtio_tx. Now we increment tx_packets/tx_bytes on success and tx_dropped on failure, so netdev stats stay accurate.
Written for commit 057e98e. Summary will update on new commits.