From 9e3f2355c452a841f98a9a9e32b71f6af157db6b Mon Sep 17 00:00:00 2001 From: David Gibson Date: Fri, 26 Jul 2024 17:20:28 +1000 Subject: tap: Don't attempt to carry on if we get a bad frame length from qemu If we receive a too-short or too-long frame from the QEMU socket, currently we try to skip it and carry on. That sounds sensible on first blush, but probably isn't wise in practice. If this happens, either (a) qemu has done something seriously unexpected, or (b) we've received corrupt data over a Unix socket. Or more likely (c), we have a bug elswhere which has put us out of sync with the stream, so we're trying to read something that's not a frame length as a frame length. Neither (b) nor (c) is really salvageable with the same stream. Case (a) might be ok, but we can no longer be confident qemu won't do something else we can't cope with. So, instead of just skipping the frame and trying to carry on, log an error and close the socket. As a bonus, establishing firm bounds on l2len early will allow simplifications to how we deal with the case where a partial frame is recv()ed. Signed-off-by: David Gibson [sbrivio: Change error message: it's not necessarily QEMU, and mention that we are resetting the connection] Signed-off-by: Stefano Brivio --- tap.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/tap.c b/tap.c index ff2b668..18aad9a 100644 --- a/tap.c +++ b/tap.c @@ -1013,7 +1013,13 @@ redo: } while (n > (ssize_t)sizeof(uint32_t)) { - ssize_t l2len = ntohl(*(uint32_t *)p); + uint32_t l2len = ntohl(*(uint32_t *)p); + + if (l2len < sizeof(struct ethhdr) || l2len > ETH_MAX_MTU) { + err("Bad frame size from guest, resetting connection"); + tap_sock_reset(c); + return; + } p += sizeof(uint32_t); n -= sizeof(uint32_t); @@ -1027,16 +1033,8 @@ redo: return; } - /* Complete the partial read above before discarding a malformed - * frame, otherwise the stream will be inconsistent. - */ - if (l2len < (ssize_t)sizeof(struct ethhdr) || - l2len > (ssize_t)ETH_MAX_MTU) - goto next; - tap_add_packet(c, l2len, p); -next: p += l2len; n -= l2len; } -- cgit v1.2.3