In _net_rx_pkt_validate_sctp_sum() we calculate a checksum that starts at the NetRxPkt::l4hdr_off offset. If this offset (which might be guest controlled) is larger than the size of the packet, the loop that walks through the iovec can walk off the end of it. The resulting out-of-bounds read in attempting to convert the iov to a buffer might cause a segfault.
We also weren't correctly checking the return values of iov_to_buf() and iov_from_buf() when reading and writing the checksum word: if the l4hdr_off claims the checksum word is half off the end of the packet these functions will return the count of how much data they were able to access, and we should fail if they don't write the entire 4 byte word. Return "checksum not valid" if the l4hdr_off is too large or the iov accesses don't do the full read/write. Note that this affects only the igb network device; the other user of this function (e1000e) does its own check on the l4 header and won't calculate a checksum if it's out of range. Cc: [email protected] Fixes: 907209e3111dd62 ("igb: Implement Rx SCTP CSO") Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4138 Signed-off-by: Peter Maydell <[email protected]> --- hw/net/net_rx_pkt.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/hw/net/net_rx_pkt.c b/hw/net/net_rx_pkt.c index f87b6f046cc..03a35f03154 100644 --- a/hw/net/net_rx_pkt.c +++ b/hw/net/net_rx_pkt.c @@ -549,19 +549,23 @@ _net_rx_pkt_validate_sctp_sum(struct NetRxPkt *pkt) uint32_t original; bool valid; - for (vec = pkt->vec; vec->iov_len < off; vec++) { + for (vec = pkt->vec; vec_len > 0 && vec->iov_len < off; vec++) { off -= vec->iov_len; vec_len--; } + if (vec_len == 0) { + return false; + } csum_off = off + 8; - if (!iov_to_buf(vec, vec_len, csum_off, &original, sizeof(original))) { + if (iov_to_buf(vec, vec_len, csum_off, + &original, sizeof(original)) != sizeof(original)) { return false; } - if (!iov_from_buf(vec, vec_len, csum_off, - &calculated, sizeof(calculated))) { + if (iov_from_buf(vec, vec_len, csum_off, + &calculated, sizeof(calculated)) != sizeof(calculated)) { return false; } -- 2.43.0
