git: 818971cc403e - main - bhyve: Fix unchecked stream I/O in RFB handler
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Thu, 19 Feb 2026 19:40:54 UTC
The branch main has been updated by markj:
URL: https://cgit.FreeBSD.org/src/commit/?id=818971cc403e78d42b77eb6c18a2d2a073e5541f
commit 818971cc403e78d42b77eb6c18a2d2a073e5541f
Author: Hayzam Sherif <hayzam@gmail.com>
AuthorDate: 2026-02-19 19:24:02 +0000
Commit: Mark Johnston <markj@FreeBSD.org>
CommitDate: 2026-02-19 19:24:07 +0000
bhyve: Fix unchecked stream I/O in RFB handler
Convert rfb_send_* helpers to return status codes and check their
results. Add missing checks for stream_read() and stream_write() returns
during the handshake in rfb_handle() to avoid acting on failed I/O.
Signed-off-by: Hayzam Sherif <hayzam@gmail.com>
Reviewed by: markj
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D55343
---
usr.sbin/bhyve/rfb.c | 76 +++++++++++++++++++++++++++++++++++++---------------
1 file changed, 54 insertions(+), 22 deletions(-)
diff --git a/usr.sbin/bhyve/rfb.c b/usr.sbin/bhyve/rfb.c
index fe6b628f94e2..c6c048d2b140 100644
--- a/usr.sbin/bhyve/rfb.c
+++ b/usr.sbin/bhyve/rfb.c
@@ -265,7 +265,7 @@ struct rfb_cuttext_msg {
uint32_t length;
};
-static void
+static int
rfb_send_server_init_msg(struct rfb_softc *rc, int cfd)
{
struct bhyvegc_image *gc_image;
@@ -289,11 +289,14 @@ rfb_send_server_init_msg(struct rfb_softc *rc, int cfd)
sinfo.pixfmt.pad[1] = 0;
sinfo.pixfmt.pad[2] = 0;
sinfo.namelen = htonl(rc->fbnamelen);
- (void)stream_write(cfd, &sinfo, sizeof(sinfo));
- (void)stream_write(cfd, rc->fbname, rc->fbnamelen);
+ if (stream_write(cfd, &sinfo, sizeof(sinfo)) <= 0)
+ return (-1);
+ if (stream_write(cfd, rc->fbname, rc->fbnamelen) <= 0)
+ return (-1);
+ return (0);
}
-static void
+static int
rfb_send_resize_update_msg(struct rfb_softc *rc, int cfd)
{
struct rfb_srvr_updt_msg supdt_msg;
@@ -303,7 +306,8 @@ rfb_send_resize_update_msg(struct rfb_softc *rc, int cfd)
supdt_msg.type = 0;
supdt_msg.pad = 0;
supdt_msg.numrects = htons(1);
- stream_write(cfd, &supdt_msg, sizeof(struct rfb_srvr_updt_msg));
+ if (stream_write(cfd, &supdt_msg, sizeof(struct rfb_srvr_updt_msg)) <= 0)
+ return (-1);
/* Rectangle header */
srect_hdr.x = htons(0);
@@ -311,10 +315,12 @@ rfb_send_resize_update_msg(struct rfb_softc *rc, int cfd)
srect_hdr.width = htons(rc->width);
srect_hdr.height = htons(rc->height);
srect_hdr.encoding = htonl(RFB_ENCODING_RESIZE);
- stream_write(cfd, &srect_hdr, sizeof(struct rfb_srvr_rect_hdr));
+ if (stream_write(cfd, &srect_hdr, sizeof(struct rfb_srvr_rect_hdr)) <= 0)
+ return (-1);
+ return (0);
}
-static void
+static int
rfb_send_extended_keyevent_update_msg(struct rfb_softc *rc, int cfd)
{
struct rfb_srvr_updt_msg supdt_msg;
@@ -324,7 +330,8 @@ rfb_send_extended_keyevent_update_msg(struct rfb_softc *rc, int cfd)
supdt_msg.type = 0;
supdt_msg.pad = 0;
supdt_msg.numrects = htons(1);
- stream_write(cfd, &supdt_msg, sizeof(struct rfb_srvr_updt_msg));
+ if (stream_write(cfd, &supdt_msg, sizeof(struct rfb_srvr_updt_msg)) <= 0)
+ return (-1);
/* Rectangle header */
srect_hdr.x = htons(0);
@@ -332,7 +339,9 @@ rfb_send_extended_keyevent_update_msg(struct rfb_softc *rc, int cfd)
srect_hdr.width = htons(rc->width);
srect_hdr.height = htons(rc->height);
srect_hdr.encoding = htonl(RFB_ENCODING_EXT_KEYEVENT);
- stream_write(cfd, &srect_hdr, sizeof(struct rfb_srvr_rect_hdr));
+ if (stream_write(cfd, &srect_hdr, sizeof(struct rfb_srvr_rect_hdr)) <= 0)
+ return (-1);
+ return (0);
}
static int
@@ -728,7 +737,10 @@ rfb_send_screen(struct rfb_softc *rc, int cfd)
rc->width = gc_image->width;
rc->height = gc_image->height;
if (rc->enc_resize_ok) {
- rfb_send_resize_update_msg(rc, cfd);
+ if (rfb_send_resize_update_msg(rc, cfd) < 0) {
+ retval = -1;
+ goto done;
+ }
rc->update_all = true;
goto done;
}
@@ -819,7 +831,10 @@ rfb_send_screen(struct rfb_softc *rc, int cfd)
goto done;
}
- rfb_send_update_header(rc, cfd, changes);
+ if (rfb_send_update_header(rc, cfd, changes) <= 0) {
+ retval = -1;
+ goto done;
+ }
/* Go through all cells, and send only changed ones */
crc_p = rc->crc_tmp;
@@ -868,7 +883,8 @@ rfb_recv_update_msg(struct rfb_softc *rc, int cfd)
return (-1);
if (rc->enc_extkeyevent_ok && (!rc->enc_extkeyevent_send)) {
- rfb_send_extended_keyevent_update_msg(rc, cfd);
+ if (rfb_send_extended_keyevent_update_msg(rc, cfd) < 0)
+ return (-1);
rc->enc_extkeyevent_send = true;
}
@@ -1045,7 +1061,8 @@ rfb_handle(struct rfb_softc *rc, int cfd)
rc->cfd = cfd;
/* 1a. Send server version */
- stream_write(cfd, vbuf, strlen(vbuf));
+ if (stream_write(cfd, vbuf, strlen(vbuf)) <= 0)
+ goto done;
/* 1b. Read client version */
len = stream_read(cfd, buf, VERSION_LENGTH);
@@ -1080,10 +1097,14 @@ rfb_handle(struct rfb_softc *rc, int cfd)
case CVERS_3_8:
buf[0] = 1;
buf[1] = auth_type;
- stream_write(cfd, buf, 2);
+ if (stream_write(cfd, buf, 2) <= 0)
+ goto done;
/* 2b. Read agreed security type */
len = stream_read(cfd, buf, 1);
+ if (len <= 0)
+ goto done;
+
if (buf[0] != auth_type) {
/* deny */
sres = htonl(1);
@@ -1094,7 +1115,8 @@ rfb_handle(struct rfb_softc *rc, int cfd)
case CVERS_3_3:
default:
be32enc(buf, auth_type);
- stream_write(cfd, buf, 4);
+ if (stream_write(cfd, buf, 4) <= 0)
+ goto done;
break;
}
@@ -1128,10 +1150,13 @@ rfb_handle(struct rfb_softc *rc, int cfd)
/* Initialize a 16-byte random challenge */
arc4random_buf(challenge, sizeof(challenge));
- stream_write(cfd, challenge, AUTH_LENGTH);
+ if (stream_write(cfd, challenge, AUTH_LENGTH) <= 0)
+ goto done;
/* Receive the 16-byte challenge response */
- stream_read(cfd, buf, AUTH_LENGTH);
+ len = stream_read(cfd, buf, AUTH_LENGTH);
+ if (len <= 0)
+ goto done;
memcpy(crypt_expected, challenge, AUTH_LENGTH);
@@ -1164,14 +1189,17 @@ rfb_handle(struct rfb_softc *rc, int cfd)
case CVERS_3_8:
report_and_done:
/* 2d. Write back a status */
- stream_write(cfd, &sres, 4);
+ if (stream_write(cfd, &sres, 4) <= 0)
+ goto done;
if (sres) {
/* 3.7 does not want string explaining cause */
if (client_ver == CVERS_3_8) {
be32enc(buf, strlen(message));
- stream_write(cfd, buf, 4);
- stream_write(cfd, message, strlen(message));
+ if (stream_write(cfd, buf, 4) <= 0)
+ goto done;
+ if (stream_write(cfd, message, strlen(message)) <= 0)
+ goto done;
}
goto done;
}
@@ -1181,7 +1209,8 @@ report_and_done:
/* for VNC auth case send status */
if (auth_type == SECURITY_TYPE_VNC_AUTH) {
/* 2d. Write back a status */
- stream_write(cfd, &sres, 4);
+ if (stream_write(cfd, &sres, 4) <= 0)
+ goto done;
}
if (sres) {
goto done;
@@ -1190,9 +1219,12 @@ report_and_done:
}
/* 3a. Read client shared-flag byte */
len = stream_read(cfd, buf, 1);
+ if (len <= 0)
+ goto done;
/* 4a. Write server-init info */
- rfb_send_server_init_msg(rc, cfd);
+ if (rfb_send_server_init_msg(rc, cfd) < 0)
+ goto done;
if (!rc->zbuf) {
rc->zbuf = malloc(RFB_ZLIB_BUFSZ + 16);