mirrors/passt
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

tap: Don't attempt to carry on if we get a bad frame length from qemu

Browse source 
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 <david@gibson.dropbear.id.au>
[sbrivio: Change error message: it's not necessarily QEMU, and mention
 that we are resetting the connection]
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
This commit is contained in:
David Gibson 2024-07-26 17:20:28 +10:00 committed by Stefano Brivio
parent a06db27c49
commit 9e3f2355c4
1 changed files with 7 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
tap.c
View file

@ -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;
}