Swatantra-66 opened a new pull request, #21522:
URL: https://github.com/apache/nuttx/pull/21522
## Summary
Fixes integer overflow vulnerabilities and datagram truncation issues in
RPMsg sockets (`net/rpmsg`):
1. **Unchecked `iov_len` accumulation overflow (`rpmsg_socket_get_iovlen`)**:
`iov_len` values (`size_t`, 64-bit on 64-bit architectures) were
accumulated into `uint32_t len` without overflow checking. If the sum exceeded
`UINT32_MAX`, it silently wrapped around. On 64-bit architectures, individual
`iov_len` values larger than `UINT32_MAX` were also silently truncated.
- For `SOCK_STREAM` (`rpmsg_socket_send_continuous`), a wrapped length
caused `while (written < len)` to terminate prematurely or not run at all,
leading to silent data loss (returning 0 or partial transfer).
- For `SOCK_DGRAM` (`rpmsg_socket_send_single`), a wrapped length could
bypass `conn->sendsize` capacity checks.
- **Fix**: Changed return type to `ssize_t` and validate `buf->iov_len >
MIN(SSIZE_MAX, UINT32_MAX) - len` on each element, returning `-EINVAL` on
overflow (consistent with merged PR #21434 and PR #21492). Handled error return
in `rpmsg_socket_send_continuous()`.
2. **Addition overflow in datagram size check (`rpmsg_socket_send_single`)**:
`uint32_t total = len + sizeof(uint32_t)` could overflow `uint32_t` when
`len > UINT32_MAX - sizeof(uint32_t)`. If `len == UINT32_MAX`, `total` wrapped
to `3`, bypassing `total > conn->sendsize`.
- **Fix**: Check `conn->sendsize < sizeof(uint32_t) || len >
conn->sendsize - sizeof(uint32_t)` before computing `total`.
3. **Transmit buffer overrun and datagram truncation
(`rpmsg_socket_send_single`)**:
When the datagram fits `conn->sendsize` but exceeds the acquired RPMsg
buffer's `ipcsize`, the existing code previously executed:
```c
total = MIN(total, ipcsize - sizeof(*msg));
len = total - sizeof(uint32_t);
```
This silently truncated the datagram payload and transmitted an
incomplete packet as a full datagram, while also risking unsigned underflow in
`total - sizeof(uint32_t)` if `ipcsize - sizeof(*msg) < sizeof(uint32_t)`.
- **Fix**: Reject the datagram with `-EMSGSIZE` and release the acquired
buffer (`rpmsg_release_tx_buffer`) if `ipcsize < sizeof(*msg) || total >
ipcsize - sizeof(*msg)`.
4. **Preserve blocking socket semantics when window space changes
(`rpmsg_socket_send_single`)**:
If a concurrent sender reduces available window space below `total` while
the TX buffer is being acquired outside the lock:
- For non-blocking sockets (`nonblock == true`): release the buffer and
return `-EAGAIN`.
- For blocking sockets (`nonblock == false`): release the buffer and loop
back to wait on `conn->sendsem` rather than failing a blocking operation with
`-EAGAIN`.
## Impact
- Prevents silent data corruption and potential buffer overruns in RPMsg
sockets.
- Preserves atomic datagram transmission guarantees and POSIX blocking
semantics.
## Testing
- **Coding Style**: Verified with `tools/nxstyle net/rpmsg/rpmsg_sockif.c`
(0 errors, 0 warnings).
- **Static Analysis & Logic Verification**:
- Validated that `iov_len` accumulation bounds to `MIN(SSIZE_MAX,
UINT32_MAX)` returning `-EINVAL`.
- Validated that subtraction check `len > conn->sendsize -
sizeof(uint32_t)` prevents addition wrap-around in `total`.
- Validated that buffer capacity check prevents datagram truncation and
unsigned underflow.
- **Affected Configurations**: `sim:rpproxy`, `sim:rpserver`,
`arm/qemu-armv7a:rpproxy`, `arm64/qemu-armv8a:rpproxy`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
