The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=818971cc403e78d42b77eb6c18a2d2a073e5541f
commit 818971cc403e78d42b77eb6c18a2d2a073e5541f Author: Hayzam Sherif <[email protected]> AuthorDate: 2026-02-19 19:24:02 +0000 Commit: Mark Johnston <[email protected]> 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 <[email protected]> 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);
