netvsp: adding LSO validation logic to handle_rndis_packet_message #2148
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds LSO (Large Send Offload) packet validation logic to the netvsp driver's packet handling function to ensure guest-provided LSO packets meet required constraints.
Key changes:
- Adds validation for LSO packets to verify they have at least two SGEs (Scatter-Gather Elements)
- Validates that the first SGE is no larger than 256 bytes (header size constraint)
- Introduces a new error type for invalid LSO packets
vm/devices/net/netvsp/src/lib.rs
Outdated
| "LSO requires at least two SGEs", | ||
| )); | ||
| } | ||
| let first_segment_size = segments.first().unwrap().len; |
There was a problem hiding this comment.
Using unwrap() is unsafe here even though the length check exists above. If segments is empty, this will panic. Consider using segments[0].len instead since the length check guarantees at least 2 elements, or use a safer pattern like let first_segment = &segments[0];.
| let first_segment_size = segments.first().unwrap().len; | |
| let first_segment_size = segments[0].len; |
vm/devices/net/netvsp/src/lib.rs
Outdated
|
|
||
| if metadata.offload_tcp_segmentation { | ||
| if segments.len() < 2 { | ||
| return Err(WorkerError::InvalidLsoPacket( |
There was a problem hiding this comment.
should this be two different error types, and we log in the error how many segments we actually got?
vm/devices/net/netvsp/src/lib.rs
Outdated
| } | ||
| let first_segment_size = segments.first().unwrap().len; | ||
| if first_segment_size > 256 { | ||
| return Err(WorkerError::InvalidLsoPacket( |
There was a problem hiding this comment.
same with this below as above?
vm/devices/net/netvsp/src/lib.rs
Outdated
| "LSO requires at least two SGEs", | ||
| )); | ||
| } | ||
| let first_segment_size = segments.first().unwrap().len; |
erfrimod
force-pushed
the
erfrimod/netvsp-validate-lso-packets
branch
from
011988b to
7fe50de
Compare
erfrimod
force-pushed
the
erfrimod/netvsp-validate-lso-packets
branch
from
7fe50de to
a92d91d
Compare
vm/devices/net/netvsp/src/lib.rs
Outdated
| )); | ||
| } | ||
| let first_segment_size = segments[0].len; | ||
| if first_segment_size > 256 { |
There was a problem hiding this comment.
This is a MANA requirement, but I dont see this as a (R)NDIS requirement here https://learn.microsoft.com/en-us/windows-hardware/drivers/network/offloading-the-segmentation-of-large-tcp-packets
There was a problem hiding this comment.
Ok, I'll remove this check.
vm/devices/net/netvsp/src/lib.rs
Outdated
| } | ||
|
|
||
| if metadata.offload_tcp_segmentation { | ||
| if segments.len() < 2 { |
There was a problem hiding this comment.
Lets define a constant for the min segment count and also use the same constant for the ndis_offload min_segement_count property for IPv4/v6
For reference, here is the spec https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/ntddndis/ns-ntddndis-_ndis_tcp_large_send_offload_v2
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…nt count constant
4 participants
netvsp should be validating the LSO packets coming from the guest