virtio/net/unixgram: Fix SO_SNDBUF size on macOS#574
Conversation
nirs
requested review from
MatiasVara,
dorindabassey,
jakecorrenti,
mtjhrc,
slp and
tylerfanelli
as code owners
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly adjusts the SO_SNDBUF size for macOS Unix datagram sockets and fixes an incorrect log message for SO_RCVBUF. The changes are logical and improve platform-specific behavior. I've included one suggestion to refactor the new constants for better maintainability by avoiding a repeated magic number.
nirs
force-pushed
the
macos-send-buffer-size
branch
from
cdd5de7 to
fe010e7
Compare
mem::size_of is a const fn, so the virtio-net header length can be a compile-time constant instead of a function call. This allows using it in const expressions, which is needed in a following change to derive the macOS socket send buffer size from MAX_BUFFER_SIZE. Assisted-by: Cursor/Claude Opus 4.6 Signed-off-by: Nir Soffer <nirsof@gmail.com>
nirs
force-pushed
the
macos-send-buffer-size
branch
from
fe010e7 to
e927043
Compare
|
Latest version defines the maximum frame size using existing constants instead of adding another magic number. /gemini review |
nirs
force-pushed
the
macos-send-buffer-size
branch
from
e927043 to
d794ebe
Compare
There was a problem hiding this comment.
Code Review
This pull request correctly fixes the SO_SNDBUF size for Unix datagram sockets on macOS, aligning it with the kernel's semantics for maximum datagram size, and refactors a function into a constant for the virtio net header length. While these changes improve correctness and code quality, a potential Guest-to-Host Denial of Service (DoS) vulnerability has been identified in the NetWorker::process_tx loop. The code currently lacks validation to ensure that the total length of a packet received from the guest is at least as large as the virtio-net header size. Sending a shorter packet could cause the host's worker thread to panic. Additionally, a copy-paste error in a log message for SO_RCVBUF has been fixed. Please address the DoS vulnerability by adding packet length validation before processing.
On macOS, Unix datagram sockets do not use the send buffer for queuing. The send buffer size determines the maximum datagram size that can be sent. Set SO_SNDBUF to the maximum frame size (65550) instead of 7 MiB to match the actual semantics. On Linux, keep the 7 MiB send buffer for actual queuing. Also fix the SO_RCVBUF warning message which incorrectly said "SO_SNDBUF". Assisted-by: Cursor/Claude Opus 4.6 Signed-off-by: Nir Soffer <nirsof@gmail.com>
nirs
force-pushed
the
macos-send-buffer-size
branch
from
d794ebe to
4a90cfe
Compare
3 participants
On macOS, Unix datagram sockets do not use the send buffer for queuing. The send buffer size determines the maximum datagram size that can be sent. Set SO_SNDBUF to the maximum frame size (65550) instead of 7 MiB to match the actual semantics.
On Linux, keep the 7 MiB send buffer for actual queuing.
Also fix the SO_RCVBUF warning message which incorrectly said "SO_SNDBUF".