netvsp: fix error handling for RNDIS multipackets with an invalid segment #2141
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug in the netvsp driver where handling multi-packet RNDIS messages could lead to duplicate TxId values being returned to the pool. The issue occurred when the first packet in a multi-packet message was processed successfully but a subsequent packet failed - the successful packet's segment would still be transmitted while the failed packet would return its TxId to the pool, causing the same TxId to be returned twice.
Key changes:
- Added logic to clear tx_segments when an error occurs during multi-packet processing
- Added comprehensive tests for multi-packet RNDIS scenarios
- Refactored existing test code to use helper methods for better maintainability
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| vm/devices/net/netvsp/src/lib.rs | Added tx_segments.clear() call in error handling path to prevent duplicate TxId pool returns |
| vm/devices/net/netvsp/src/test.rs | Added helper methods and new tests for multi-packet RNDIS scenarios, including error cases |
erfrimod
force-pushed
the
erfrimod/fix-tx-pending-crash
branch
from
d542ec7 to
0a33262
Compare
| }) | ||
| } else { | ||
| None | ||
| }; |
There was a problem hiding this comment.
| }; | |
| let mut pending_control_messages = state.primary.map(|primary| PendingControlMessages { | |
| total_len: primary.control_messages_len, | |
| control_messages: Vec::new(), | |
| }); |
| let num_packets = match result { | ||
| Ok(num_packets) => num_packets, | ||
| Ok(num_packets) => { | ||
| if let Some(pending_control_messages) = pending_control_messages { |
There was a problem hiding this comment.
if let Some(pending_control_messages) = pending_control_messages.as_mut() {
| message_type: u32, | ||
| mut reader: PacketReader<'_>, | ||
| segments: &mut Vec<TxSegment>, | ||
| pending_control_messages: &mut Option<&mut PendingControlMessages>, |
There was a problem hiding this comment.
pending_control_messages: &mut Option,
3 participants
When netvsp handles a multi-packet RNDIS packet, it loops over each packet. If the first packet processes without error, it is added to tx_segments. If a subsequent packet encounters an error during parsing, the reserved TxId is returned to the pool when the packet is completed as an error. But the segment remaining in tx_segments gets sent by transmit_pending_segments() and that completion adds a duplicate of TxId to the pool. In the future, multiple packets can be assigned the same TxId.