untrusted comment: verify with openbsd-68-base.pub

untrusted comment: verify with openbsd-68-base.pub
RWQZj25CSG5R2vcQVSpjuOG3lYIbuZWDGkvuty/hqr+WKQaGcV0GT2QDBH8MAIikXhkmnBMamr0tjq8KwoNM37r8Ac0alaUOeAg=

OpenBSD 6.8 errata 020, May 18, 2021:

vmd guest virtio drivers could cause stack overflows by crafting
invalid virtio descriptor lengths.

Apply by doing:
signify -Vep /etc/signify/openbsd-68-base.pub -x 020_vmd.patch.sig \
-m - | (cd /usr/src && patch -p0)

And then rebuild and install vmd(8) and vmctl(8):
cd /usr/src/usr.sbin/vmd
make obj
make
make install
cd /usr/src/usr.sbin/vmctl
make obj
make
make install

Index: usr.sbin/vmd/vioscsi.c
===================================================================
RCS file: /cvs/src/usr.sbin/vmd/vioscsi.c,v
diff -u -p -r1.14 vioscsi.c
--- usr.sbin/vmd/vioscsi.c 3 Sep 2020 13:11:49 -0000 1.14
+++ usr.sbin/vmd/vioscsi.c 13 May 2021 18:56:16 -0000
@@ -186,10 +186,16 @@ vioscsi_free_info(struct ioinfo *info)
}

static struct ioinfo *
-vioscsi_start_read(struct vioscsi_dev *dev, off_t block, ssize_t n_blocks)
+vioscsi_start_read(struct vioscsi_dev *dev, off_t block, size_t n_blocks)
{
struct ioinfo *info;

+ /* Limit to 64M for now */
+ if (n_blocks * VIOSCSI_BLOCK_SIZE_CDROM > (1 << 26)) {
+ log_warnx("%s: read size exceeded 64M", __func__);
+ return (NULL);
+ }
+
info = calloc(1, sizeof(*info));
if (!info)
goto nomem;
@@ -237,7 +243,7 @@ vioscsi_handle_tur(struct vioscsi_dev *d

vioscsi_prepare_resp(&resp, VIRTIO_SCSI_S_OK, SCSI_OK, 0, 0, 0);

- if (write_mem(acct->resp_desc->addr, &resp, acct->resp_desc->len)) {
+ if (write_mem(acct->resp_desc->addr, &resp, sizeof(resp))) {
log_warnx("%s: unable to write OK resp status data @ 0x%llx",
__func__, acct->resp_desc->addr);
} else {
@@ -295,7 +301,7 @@ vioscsi_handle_inquiry(struct vioscsi_de
"idx %d req_idx %d global_idx %d", __func__, acct->resp_desc->addr,
acct->resp_desc->len, acct->resp_idx, acct->req_idx, acct->idx);

- if (write_mem(acct->resp_desc->addr, &resp, acct->resp_desc->len)) {
+ if (write_mem(acct->resp_desc->addr, &resp, sizeof(resp))) {
log_warnx("%s: unable to write OK resp status data @ 0x%llx",
__func__, acct->resp_desc->addr);
goto free_inq;
@@ -310,7 +316,8 @@ vioscsi_handle_inquiry(struct vioscsi_de
__func__, acct->resp_desc->addr, acct->resp_desc->len,
acct->resp_idx, acct->req_idx, acct->idx);

- if (write_mem(acct->resp_desc->addr, inq_data, acct->resp_desc->len)) {
+ if (write_mem(acct->resp_desc->addr, inq_data,
+ sizeof(struct scsi_inquiry_data))) {
log_warnx("%s: unable to write inquiry"
" response to gpa @ 0x%llx",
__func__, acct->resp_desc->addr);
@@ -337,7 +344,7 @@ vioscsi_handle_mode_sense(struct vioscsi
uint8_t mode_page_ctl;
uint8_t mode_page_code;
uint8_t *mode_reply;
- uint8_t mode_reply_len;
+ uint8_t mode_reply_len = 0;
struct scsi_mode_sense *mode_sense;

memset(&resp, 0, sizeof(resp));
@@ -356,7 +363,7 @@ vioscsi_handle_mode_sense(struct vioscsi
* mode sense header is 4 bytes followed
* by a variable page
* ERR_RECOVERY_PAGE is 12 bytes
- * CDVD_CAPABILITIES_PAGE is 27 bytes
+ * CDVD_CAPABILITIES_PAGE is 32 bytes
*/
switch (mode_page_code) {
case ERR_RECOVERY_PAGE:
@@ -376,7 +383,7 @@ vioscsi_handle_mode_sense(struct vioscsi
*(mode_reply + 7) = MODE_READ_RETRY_COUNT;
break;
case CDVD_CAPABILITIES_PAGE:
- mode_reply_len = 31;
+ mode_reply_len = 36;
mode_reply =
(uint8_t*)calloc(mode_reply_len, sizeof(uint8_t));
if (mode_reply == NULL)
@@ -403,11 +410,10 @@ vioscsi_handle_mode_sense(struct vioscsi

DPRINTF("%s: writing resp to 0x%llx size %d "
"at local idx %d req_idx %d global_idx %d",
- __func__, acct->resp_desc->addr, acct->resp_desc->len,
+ __func__, acct->resp_desc->addr, mode_reply_len,
acct->resp_idx, acct->req_idx, acct->idx);

- if (write_mem(acct->resp_desc->addr, &resp,
- acct->resp_desc->len)) {
+ if (write_mem(acct->resp_desc->addr, &resp, sizeof(resp))) {
log_warnx("%s: unable to write OK"
" resp status data @ 0x%llx",
__func__, acct->resp_desc->addr);
@@ -421,12 +427,11 @@ vioscsi_handle_mode_sense(struct vioscsi

DPRINTF("%s: writing mode_reply to 0x%llx "
"size %d at local idx %d req_idx %d "
- "global_idx %d",__func__, acct->resp_desc->addr,
- acct->resp_desc->len, acct->resp_idx, acct->req_idx,
- acct->idx);
+ "global_idx %d", __func__, acct->resp_desc->addr,
+ mode_reply_len, acct->resp_idx, acct->req_idx, acct->idx);

if (write_mem(acct->resp_desc->addr, mode_reply,
- acct->resp_desc->len)) {
+ mode_reply_len)) {
log_warnx("%s: unable to write "
"mode_reply to gpa @ 0x%llx",
__func__, acct->resp_desc->addr);
@@ -452,8 +457,7 @@ mode_sense_error:
acct->resp_desc = vioscsi_next_ring_desc(acct->desc,
acct->req_desc, &(acct->resp_idx));

- if (write_mem(acct->resp_desc->addr, &resp,
- acct->resp_desc->len)) {
+ if (write_mem(acct->resp_desc->addr, &resp, sizeof(resp))) {
log_warnx("%s: unable to set ERR status data @ 0x%llx",
__func__, acct->resp_desc->addr);
goto mode_sense_out;
@@ -478,7 +482,7 @@ vioscsi_handle_mode_sense_big(struct vio
uint8_t mode_page_ctl;
uint8_t mode_page_code;
uint8_t *mode_reply;
- uint8_t mode_reply_len;
+ uint8_t mode_reply_len = 0;
uint16_t mode_sense_len;
struct scsi_mode_sense_big *mode_sense_10;

@@ -499,7 +503,7 @@ vioscsi_handle_mode_sense_big(struct vio
* mode sense header is 8 bytes followed
* by a variable page
* ERR_RECOVERY_PAGE is 12 bytes
- * CDVD_CAPABILITIES_PAGE is 27 bytes
+ * CDVD_CAPABILITIES_PAGE is 32 bytes
*/
switch (mode_page_code) {
case ERR_RECOVERY_PAGE:
@@ -519,7 +523,7 @@ vioscsi_handle_mode_sense_big(struct vio
*(mode_reply + 11) = MODE_READ_RETRY_COUNT;
break;
case CDVD_CAPABILITIES_PAGE:
- mode_reply_len = 35;
+ mode_reply_len = 40;
mode_reply =
(uint8_t*)calloc(mode_reply_len, sizeof(uint8_t));
if (mode_reply == NULL)
@@ -549,8 +553,7 @@ vioscsi_handle_mode_sense_big(struct vio
__func__, acct->resp_desc->addr, acct->resp_desc->len,
acct->resp_idx, acct->req_idx, acct->idx);

- if (write_mem(acct->resp_desc->addr, &resp,
- acct->resp_desc->len)) {
+ if (write_mem(acct->resp_desc->addr, &resp, sizeof(resp))) {
log_warnx("%s: unable to write OK"
" resp status data @ 0x%llx",
__func__, acct->resp_desc->addr);
@@ -564,11 +567,11 @@ vioscsi_handle_mode_sense_big(struct vio

DPRINTF("%s: writing mode_reply to 0x%llx "
"size %d at local idx %d req_idx %d global_idx %d",
- __func__, acct->resp_desc->addr, acct->resp_desc->len,
+ __func__, acct->resp_desc->addr, mode_reply_len,
acct->resp_idx, acct->req_idx, acct->idx);

if (write_mem(acct->resp_desc->addr, mode_reply,
- acct->resp_desc->len)) {
+ mode_reply_len)) {
log_warnx("%s: unable to write "
"mode_reply to gpa @ 0x%llx",
__func__, acct->resp_desc->addr);
@@ -594,8 +597,7 @@ mode_sense_big_error:
acct->resp_desc = vioscsi_next_ring_desc(acct->desc,
acct->req_desc, &(acct->resp_idx));

- if (write_mem(acct->resp_desc->addr, &resp,
- acct->resp_desc->len)) {
+ if (write_mem(acct->resp_desc->addr, &resp, sizeof(resp))) {
log_warnx("%s: unable to set ERR status data @ 0x%llx",
__func__, acct->resp_desc->addr);
goto mode_sense_big_out;
@@ -666,7 +668,7 @@ vioscsi_handle_read_capacity(struct vios
__func__, acct->resp_desc->addr, acct->resp_desc->len,
acct->resp_idx, acct->req_idx, acct->idx);

- if (write_mem(acct->resp_desc->addr, &resp, acct->resp_desc->len)) {
+ if (write_mem(acct->resp_desc->addr, &resp, sizeof(resp))) {
log_warnx("%s: unable to write OK resp status data @ 0x%llx",
__func__, acct->resp_desc->addr);
goto free_read_capacity;
@@ -682,7 +684,7 @@ vioscsi_handle_read_capacity(struct vios
acct->resp_idx, acct->req_idx, acct->idx);

if (write_mem(acct->resp_desc->addr, r_cap_data,
- acct->resp_desc->len)) {
+ sizeof(struct scsi_read_cap_data))) {
log_warnx("%s: unable to write read_cap_data"
" response to gpa @ 0x%llx",
__func__, acct->resp_desc->addr);
@@ -741,7 +743,7 @@ vioscsi_handle_read_capacity_16(struct v
__func__, acct->resp_desc->addr, acct->resp_desc->len,
acct->resp_idx, acct->req_idx, acct->idx);

- if (write_mem(acct->resp_desc->addr, &resp, acct->resp_desc->len)) {
+ if (write_mem(acct->resp_desc->addr, &resp, sizeof(resp))) {
log_warnx("%s: unable to write OK resp status "
"data @ 0x%llx", __func__, acct->resp_desc->addr);
goto free_read_capacity_16;
@@ -757,7 +759,7 @@ vioscsi_handle_read_capacity_16(struct v
acct->resp_idx, acct->req_idx, acct->idx);

if (write_mem(acct->resp_desc->addr, r_cap_data_16,
- acct->resp_desc->len)) {
+ sizeof(struct scsi_read_cap_data_16))) {
log_warnx("%s: unable to write read_cap_data_16"
" response to gpa @ 0x%llx",
__func__, acct->resp_desc->addr);
@@ -804,8 +806,7 @@ vioscsi_handle_report_luns(struct vioscs
acct->resp_desc = vioscsi_next_ring_desc(acct->desc,
acct->req_desc, &(acct->resp_idx));

- if (write_mem(acct->resp_desc->addr, &resp,
- acct->resp_desc->len)) {
+ if (write_mem(acct->resp_desc->addr, &resp, sizeof(resp))) {
log_warnx("%s: unable to set ERR "
"status data @ 0x%llx", __func__,
acct->resp_desc->addr);
@@ -820,7 +821,7 @@ vioscsi_handle_report_luns(struct vioscs

}

- reply_rpl = calloc(1, sizeof(*reply_rpl));
+ reply_rpl = calloc(1, sizeof(struct vioscsi_report_luns_data));

if (reply_rpl == NULL) {
log_warnx("%s: cannot alloc reply_rpl", __func__);
@@ -841,7 +842,7 @@ vioscsi_handle_report_luns(struct vioscs
"idx %d req_idx %d global_idx %d", __func__, acct->resp_desc->addr,
acct->resp_desc->len, acct->resp_idx, acct->req_idx, acct->idx);

- if (write_mem(acct->resp_desc->addr, &resp, acct->resp_desc->len)) {
+ if (write_mem(acct->resp_desc->addr, &resp, sizeof(resp))) {
log_warnx("%s: unable to write OK resp status data @ 0x%llx",
__func__, acct->resp_desc->addr);
goto free_rpl;
@@ -856,7 +857,8 @@ vioscsi_handle_report_luns(struct vioscs
__func__, acct->resp_desc->addr, acct->resp_desc->len,
acct->resp_idx, acct->req_idx, acct->idx);

- if (write_mem(acct->resp_desc->addr, reply_rpl, acct->resp_desc->len)) {
+ if (write_mem(acct->resp_desc->addr, reply_rpl,
+ sizeof(struct vioscsi_report_luns_data))) {
log_warnx("%s: unable to write reply_rpl"
" response to gpa @ 0x%llx",
__func__, acct->resp_desc->addr);
@@ -906,8 +908,7 @@ vioscsi_handle_read_6(struct vioscsi_dev
acct->resp_desc = vioscsi_next_ring_desc(acct->desc,
acct->req_desc, &(acct->resp_idx));

- if (write_mem(acct->resp_desc->addr, &resp,
- acct->resp_desc->len)) {
+ if (write_mem(acct->resp_desc->addr, &resp, sizeof(resp))) {
log_warnx("%s: unable to set ERR "
"status data @ 0x%llx", __func__,
acct->resp_desc->addr);
@@ -942,8 +943,7 @@ vioscsi_handle_read_6(struct vioscsi_dev
acct->resp_desc = vioscsi_next_ring_desc(acct->desc,
acct->req_desc, &(acct->resp_idx));

- if (write_mem(acct->resp_desc->addr, &resp,
- acct->resp_desc->len)) {
+ if (write_mem(acct->resp_desc->addr, &resp, sizeof(resp))) {
log_warnx("%s: unable to set ERR "
"status data @ 0x%llx", __func__,
acct->resp_desc->addr);
@@ -969,7 +969,7 @@ vioscsi_handle_read_6(struct vioscsi_dev
__func__, acct->resp_desc->addr, acct->resp_desc->len,
acct->resp_idx, acct->req_idx, acct->idx);

- if (write_mem(acct->resp_desc->addr, &resp, acct->resp_desc->len)) {
+ if (write_mem(acct->resp_desc->addr, &resp, sizeof(resp))) {
log_warnx("%s: unable to write OK resp status "
"data @ 0x%llx", __func__, acct->resp_desc->addr);
goto free_read_6;
@@ -984,8 +984,7 @@ vioscsi_handle_read_6(struct vioscsi_dev
__func__, acct->resp_desc->addr, acct->resp_desc->len,
acct->resp_idx, acct->req_idx, acct->idx);

- if (write_mem(acct->resp_desc->addr, read_buf,
- acct->resp_desc->len)) {
+ if (write_mem(acct->resp_desc->addr, read_buf, info->len)) {
log_warnx("%s: unable to write read_buf to gpa @ 0x%llx",
__func__, acct->resp_desc->addr);
} else {
@@ -1014,6 +1013,7 @@ vioscsi_handle_read_10(struct vioscsi_de
off_t chunk_offset;
struct ioinfo *info;
struct scsi_rw_10 *read_10;
+ size_t chunk_len = 0;

memset(&resp, 0, sizeof(resp));
read_10 = (struct scsi_rw_10 *)(req->cdb);
@@ -1037,8 +1037,7 @@ vioscsi_handle_read_10(struct vioscsi_de
acct->resp_desc = vioscsi_next_ring_desc(acct->desc,
acct->req_desc, &(acct->resp_idx));

- if (write_mem(acct->resp_desc->addr, &resp,
- acct->resp_desc->len)) {
+ if (write_mem(acct->resp_desc->addr, &resp, sizeof(resp))) {
log_warnx("%s: unable to set ERR status data @ 0x%llx",
__func__, acct->resp_desc->addr);
} else {
@@ -1072,8 +1071,7 @@ vioscsi_handle_read_10(struct vioscsi_de
acct->resp_desc = vioscsi_next_ring_desc(acct->desc,
acct->req_desc, &(acct->resp_idx));

- if (write_mem(acct->resp_desc->addr, &resp,
- acct->resp_desc->len)) {
+ if (write_mem(acct->resp_desc->addr, &resp, sizeof(resp))) {
log_warnx("%s: unable to set ERR status data @ 0x%llx",
__func__, acct->resp_desc->addr);
} else {
@@ -1098,7 +1096,7 @@ vioscsi_handle_read_10(struct vioscsi_de
__func__, acct->resp_desc->addr, acct->resp_desc->len,
acct->resp_idx, acct->req_idx, acct->idx);

- if (write_mem(acct->resp_desc->addr, &resp, acct->resp_desc->len)) {
+ if (write_mem(acct->resp_desc->addr, &resp, sizeof(resp))) {
log_warnx("%s: unable to write OK resp status "
"data @ 0x%llx", __func__, acct->resp_desc->addr);
goto free_read_10;
@@ -1120,8 +1118,16 @@ vioscsi_handle_read_10(struct vioscsi_de
__func__, acct->resp_desc->addr, acct->resp_desc->len,
acct->resp_idx, acct->req_idx, acct->idx);

- if (write_mem(acct->resp_desc->addr,
- read_buf + chunk_offset, acct->resp_desc->len)) {
+ /* Check we don't read beyond read_buf boundaries. */
+ if (acct->resp_desc->len > info->len - chunk_offset) {
+ log_warnx("%s: descriptor length beyond read_buf len",
+ __func__);
+ chunk_len = info->len - chunk_offset;
+ } else
+ chunk_len = acct->resp_desc->len;
+
+ if (write_mem(acct->resp_desc->addr, read_buf + chunk_offset,
+ chunk_len)) {
log_warnx("%s: unable to write read_buf"
" to gpa @ 0x%llx", __func__,
acct->resp_desc->addr);
@@ -1164,7 +1170,7 @@ vioscsi_handle_prevent_allow(struct vios

dev->locked = dev->locked ? 0 : 1;

- if (write_mem(acct->resp_desc->addr, &resp, acct->resp_desc->len)) {
+ if (write_mem(acct->resp_desc->addr, &resp, sizeof(resp))) {
log_warnx("%s: unable to write OK resp status data @ 0x%llx",
__func__, acct->resp_desc->addr);
} else {
@@ -1193,7 +1199,8 @@ vioscsi_handle_mechanism_status(struct v
mech_status_len = (uint16_t)_2btol(mech_status->length);
DPRINTF("%s: MECH_STATUS Len %u", __func__, mech_status_len);

- mech_status_header = calloc(1, sizeof(*mech_status_header));
+ mech_status_header = calloc(1,
+ sizeof(struct scsi_mechanism_status_header));

if (mech_status_header == NULL)
goto mech_out;
@@ -1206,7 +1213,7 @@ vioscsi_handle_mechanism_status(struct v
acct->resp_desc = vioscsi_next_ring_desc(acct->desc,
acct->req_desc, &(acct->resp_idx));

- if (write_mem(acct->resp_desc->addr, &resp, acct->resp_desc->len)) {
+ if (write_mem(acct->resp_desc->addr, &resp, sizeof(resp))) {
log_warnx("%s: unable to set ERR status data @ 0x%llx",
__func__, acct->resp_desc->addr);
goto free_mech;
@@ -1217,7 +1224,7 @@ vioscsi_handle_mechanism_status(struct v
&(acct->resp_idx));

if (write_mem(acct->resp_desc->addr, mech_status_header,
- acct->resp_desc->len)) {
+ sizeof(struct scsi_mechanism_status_header))) {
log_warnx("%s: unable to write "
"mech_status_header response to "
"gpa @ 0x%llx",
@@ -1272,8 +1279,7 @@ vioscsi_handle_read_toc(struct vioscsi_d
acct->resp_desc = vioscsi_next_ring_desc(acct->desc,
acct->req_desc, &(acct->resp_idx));

- if (write_mem(acct->resp_desc->addr, &resp,
- acct->resp_desc->len)) {
+ if (write_mem(acct->resp_desc->addr, &resp, sizeof(resp))) {
log_warnx("%s: unable to set ERR status data @ 0x%llx",
__func__, acct->resp_desc->addr);
goto read_toc_out;
@@ -1347,7 +1353,7 @@ vioscsi_handle_read_toc(struct vioscsi_d
__func__, acct->resp_desc->addr, acct->resp_desc->len,
acct->resp_idx, acct->req_idx, acct->idx);

- if (write_mem(acct->resp_desc->addr, &resp, acct->resp_desc->len)) {
+ if (write_mem(acct->resp_desc->addr, &resp, sizeof(resp))) {
log_warnx("%s: unable to write OK resp status data @ 0x%llx",
__func__, acct->resp_desc->addr);
goto read_toc_out;
@@ -1362,8 +1368,7 @@ vioscsi_handle_read_toc(struct vioscsi_d
__func__, acct->resp_desc->addr, acct->resp_desc->len,
acct->resp_idx, acct->req_idx, acct->idx);

- if (write_mem(acct->resp_desc->addr, toc_data,
- acct->resp_desc->len)) {
+ if (write_mem(acct->resp_desc->addr, toc_data, sizeof(toc_data))) {
log_warnx("%s: unable to write toc descriptor data @ 0x%llx",
__func__, acct->resp_desc->addr);
} else {
@@ -1400,8 +1405,7 @@ vioscsi_handle_read_disc_info(struct vio
acct->resp_desc = vioscsi_next_ring_desc(acct->desc,
acct->req_desc, &(acct->resp_idx));

- if (write_mem(acct->resp_desc->addr, &resp,
- acct->resp_desc->len)) {
+ if (write_mem(acct->resp_desc->addr, &resp, sizeof(resp))) {
log_warnx("%s: unable to set ERR status data @ 0x%llx",
__func__, acct->resp_desc->addr);
} else {
@@ -1441,8 +1445,7 @@ vioscsi_handle_gesn(struct vioscsi_dev *
acct->resp_desc = vioscsi_next_ring_desc(acct->desc,
acct->req_desc, &(acct->resp_idx));

- if (write_mem(acct->resp_desc->addr, &resp,
- acct->resp_desc->len)) {
+ if (write_mem(acct->resp_desc->addr, &resp, sizeof(resp))) {
log_warnx("%s: unable to set ERR status data @ 0x%llx",
__func__, acct->resp_desc->addr);
goto gesn_out;
@@ -1480,7 +1483,7 @@ vioscsi_handle_gesn(struct vioscsi_dev *
__func__, acct->resp_desc->addr, acct->resp_desc->len,
acct->resp_idx, acct->req_idx, acct->idx);

- if (write_mem(acct->resp_desc->addr, &resp, acct->resp_desc->len)) {
+ if (write_mem(acct->resp_desc->addr, &resp, sizeof(resp))) {
log_warnx("%s: unable to write OK resp status "
"data @ 0x%llx", __func__, acct->resp_desc->addr);
goto gesn_out;
@@ -1495,8 +1498,7 @@ vioscsi_handle_gesn(struct vioscsi_dev *
__func__, acct->resp_desc->addr, acct->resp_desc->len,
acct->resp_idx, acct->req_idx, acct->idx);

- if (write_mem(acct->resp_desc->addr, gesn_reply,
- acct->resp_desc->len)) {
+ if (write_mem(acct->resp_desc->addr, gesn_reply, sizeof(gesn_reply))) {
log_warnx("%s: unable to write gesn_reply"
" response to gpa @ 0x%llx",
__func__, acct->resp_desc->addr);
@@ -1625,8 +1627,7 @@ vioscsi_handle_get_config(struct vioscsi
__func__, acct->resp_desc->addr, acct->resp_desc->len,
acct->resp_idx, acct->req_idx, acct->idx);

- if (write_mem(acct->resp_desc->addr, &resp,
- acct->resp_desc->len)) {
+ if (write_mem(acct->resp_desc->addr, &resp, sizeof(resp))) {
log_warnx("%s: unable to set Ok status data @ 0x%llx",
__func__, acct->resp_desc->addr);
goto free_get_config;
@@ -1642,7 +1643,7 @@ vioscsi_handle_get_config(struct vioscsi
acct->resp_idx, acct->req_idx, acct->idx);

if (write_mem(acct->resp_desc->addr, get_conf_reply,
- acct->resp_desc->len)) {
+ G_CONFIG_REPLY_SIZE)) {
log_warnx("%s: unable to write get_conf_reply"
" response to gpa @ 0x%llx",
__func__, acct->resp_desc->addr);
@@ -2081,7 +2082,7 @@ vioscsi_notifyq(struct vioscsi_dev *dev)
{
uint64_t q_gpa;
uint32_t vr_sz;
- int ret;
+ int cnt, ret;
char *vr;
struct virtio_scsi_req_hdr req;
struct virtio_scsi_res_hdr resp;
@@ -2123,8 +2124,15 @@ vioscsi_notifyq(struct vioscsi_dev *dev)
goto out;
}

+ cnt = 0;
while (acct.idx != (acct.avail->idx & VIOSCSI_QUEUE_MASK)) {

+ /* Guard against infinite descriptor chains */
+ if (++cnt >= VIOSCSI_QUEUE_SIZE) {
+ log_warnx("%s: invalid descriptor table", __func__);
+ goto out;
+ }
+
acct.req_idx = acct.avail->ring[acct.idx] & VIOSCSI_QUEUE_MASK;
acct.req_desc = &(acct.desc[acct.req_idx]);

@@ -2138,7 +2146,7 @@ vioscsi_notifyq(struct vioscsi_dev *dev)
}

/* Read command from descriptor ring */
- if (read_mem(acct.req_desc->addr, &req, acct.req_desc->len)) {
+ if (read_mem(acct.req_desc->addr, &req, sizeof(req))) {
log_warnx("%s: command read_mem error @ 0x%llx",
__func__, acct.req_desc->addr);
goto out;
@@ -2170,8 +2178,13 @@ vioscsi_notifyq(struct vioscsi_dev *dev)
vioscsi_prepare_resp(&resp,
VIRTIO_SCSI_S_BAD_TARGET, SCSI_OK, 0, 0, 0);

+ if (acct.resp_desc->len > sizeof(resp)) {
+ log_warnx("%s: invalid descriptor length",
+ __func__);
+ goto out;
+ }
if (write_mem(acct.resp_desc->addr, &resp,
- acct.resp_desc->len)) {
+ sizeof(resp))) {
log_warnx("%s: unable to write BAD_TARGET"
" resp status data @ 0x%llx",
__func__, acct.resp_desc->addr);
Index: usr.sbin/vmd/virtio.c
===================================================================
RCS file: /cvs/src/usr.sbin/vmd/virtio.c,v
diff -u -p -r1.82 virtio.c
--- usr.sbin/vmd/virtio.c 11 Dec 2019 06:45:16 -0000 1.82
+++ usr.sbin/vmd/virtio.c 13 May 2021 18:56:16 -0000
@@ -30,6 +30,7 @@
#include <net/if.h>
#include <netinet/in.h>
#include <netinet/if_ether.h>
+#include <netinet/ip.h>

#include <errno.h>
#include <event.h>
@@ -86,6 +87,8 @@ vioblk_cmd_name(uint32_t type)
static void
dump_descriptor_chain(struct vring_desc *desc, int16_t dxx)
{
+ unsigned int cnt = 0;
+
log_debug("descriptor chain @ %d", dxx);
do {
log_debug("desc @%d addr/len/flags/next = 0x%llx / 0x%x "
@@ -96,6 +99,15 @@ dump_descriptor_chain(struct vring_desc
desc[dxx].flags,
desc[dxx].next);
dxx = desc[dxx].next;
+
+ /*
+ * Dump up to the max number of descriptor for the largest
+ * queue we support, which currently is VIONET_QUEUE_SIZE.
+ */
+ if (++cnt >= VIONET_QUEUE_SIZE) {
+ log_warnx("%s: descriptor table invalid", __func__);
+ return;
+ }
} while (desc[dxx].flags & VRING_DESC_F_NEXT);

log_debug("desc @%d addr/len/flags/next = 0x%llx / 0x%x / 0x%x "
@@ -220,8 +232,7 @@ viornd_notifyq(void)

if (rnd_data != NULL) {
arc4random_buf(rnd_data, desc[avail->ring[aidx]].len);
- if (write_mem(desc[avail->ring[aidx]].addr,
- rnd_data, desc[avail->ring[aidx]].len)) {
+ if (write_mem(desc[avail->ring[aidx]].addr, rnd_data, sz)) {
log_warnx("viornd: can't write random data @ "
"0x%llx",
desc[avail->ring[aidx]].addr);
@@ -349,10 +360,16 @@ vioblk_free_info(struct ioinfo *info)
}

static struct ioinfo *
-vioblk_start_read(struct vioblk_dev *dev, off_t sector, ssize_t sz)
+vioblk_start_read(struct vioblk_dev *dev, off_t sector, size_t sz)
{
struct ioinfo *info;

+ /* Limit to 64M for now */
+ if (sz > (1 << 26)) {
+ log_warnx("%s: read size exceeded 64M", __func__);
+ return (NULL);
+ }
+
info = calloc(1, sizeof(*info));
if (!info)
goto nomem;
@@ -393,9 +410,16 @@ vioblk_start_write(struct vioblk_dev *de
{
struct ioinfo *info;

+ /* Limit to 64M for now */
+ if (len > (1 << 26)) {
+ log_warnx("%s: write size exceeded 64M", __func__);
+ return (NULL);
+ }
+
info = calloc(1, sizeof(*info));
if (!info)
goto nomem;
+
info->buf = malloc(len);
if (info->buf == NULL)
goto nomem;
@@ -403,7 +427,7 @@ vioblk_start_write(struct vioblk_dev *de
info->offset = sector * VIRTIO_BLK_SECTOR_SIZE;
info->file = &dev->file;

- if (read_mem(addr, info->buf, len)) {
+ if (read_mem(addr, info->buf, info->len)) {
vioblk_free_info(info);
return NULL;
}
@@ -440,7 +463,7 @@ vioblk_notifyq(struct vioblk_dev *dev)
uint32_t vr_sz;
uint16_t idx, cmd_desc_idx, secdata_desc_idx, ds_desc_idx;
uint8_t ds;
- int ret;
+ int cnt, ret;
off_t secbias;
char *vr;
struct vring_desc *desc, *cmd_desc, *secdata_desc, *ds_desc;
@@ -495,7 +518,7 @@ vioblk_notifyq(struct vioblk_dev *dev)
}

/* Read command from descriptor ring */
- if (read_mem(cmd_desc->addr, &cmd, cmd_desc->len)) {
+ if (read_mem(cmd_desc->addr, &cmd, sizeof(cmd))) {
log_warnx("vioblk: command read_mem error @ 0x%llx",
cmd_desc->addr);
goto out;
@@ -513,14 +536,14 @@ vioblk_notifyq(struct vioblk_dev *dev)
goto out;
}

+ cnt = 0;
secbias = 0;
do {
struct ioinfo *info;
const uint8_t *secdata;

info = vioblk_start_read(dev,
- cmd.sector + secbias,
- (ssize_t)secdata_desc->len);
+ cmd.sector + secbias, secdata_desc->len);

/* read the data, use current data descriptor */
secdata = vioblk_finish_read(info);
@@ -532,7 +555,7 @@ vioblk_notifyq(struct vioblk_dev *dev)
}

if (write_mem(secdata_desc->addr, secdata,
- secdata_desc->len)) {
+ secdata_desc->len)) {
log_warnx("can't write sector "
"data to gpa @ 0x%llx",
secdata_desc->addr);
@@ -549,13 +572,20 @@ vioblk_notifyq(struct vioblk_dev *dev)
secdata_desc_idx = secdata_desc->next &
VIOBLK_QUEUE_MASK;
secdata_desc = &desc[secdata_desc_idx];
+
+ /* Guard against infinite chains */
+ if (++cnt >= VIOBLK_QUEUE_SIZE) {
+ log_warnx("%s: descriptor table "
+ "invalid", __func__);
+ goto out;
+ }
} while (secdata_desc->flags & VRING_DESC_F_NEXT);

ds_desc_idx = secdata_desc_idx;
ds_desc = secdata_desc;

ds = VIRTIO_BLK_S_OK;
- if (write_mem(ds_desc->addr, &ds, ds_desc->len)) {
+ if (write_mem(ds_desc->addr, &ds, sizeof(ds))) {
log_warnx("can't write device status data @ "
"0x%llx", ds_desc->addr);
dump_descriptor_chain(desc, cmd_desc_idx);
@@ -594,6 +624,7 @@ vioblk_notifyq(struct vioblk_dev *dev)
goto out;
}

+ cnt = 0;
secbias = 0;
do {
struct ioinfo *info;
@@ -626,13 +657,20 @@ vioblk_notifyq(struct vioblk_dev *dev)
secdata_desc_idx = secdata_desc->next &
VIOBLK_QUEUE_MASK;
secdata_desc = &desc[secdata_desc_idx];
+
+ /* Guard against infinite chains */
+ if (++cnt >= VIOBLK_QUEUE_SIZE) {
+ log_warnx("%s: descriptor table "
+ "invalid", __func__);
+ goto out;
+ }
} while (secdata_desc->flags & VRING_DESC_F_NEXT);

ds_desc_idx = secdata_desc_idx;
ds_desc = secdata_desc;

ds = VIRTIO_BLK_S_OK;
- if (write_mem(ds_desc->addr, &ds, ds_desc->len)) {
+ if (write_mem(ds_desc->addr, &ds, sizeof(ds))) {
log_warnx("wr vioblk: can't write device "
"status data @ 0x%llx", ds_desc->addr);
dump_descriptor_chain(desc, cmd_desc_idx);
@@ -658,7 +696,7 @@ vioblk_notifyq(struct vioblk_dev *dev)
ds_desc = &desc[ds_desc_idx];

ds = VIRTIO_BLK_S_OK;
- if (write_mem(ds_desc->addr, &ds, ds_desc->len)) {
+ if (write_mem(ds_desc->addr, &ds, sizeof(ds))) {
log_warnx("fl vioblk: "
"can't write device status "
"data @ 0x%llx", ds_desc->addr);
@@ -1075,7 +1113,7 @@ vionet_update_qs(struct vionet_dev *dev)
* Must be called with dev->mutex acquired.
*/
int
-vionet_enq_rx(struct vionet_dev *dev, char *pkt, ssize_t sz, int *spc)
+vionet_enq_rx(struct vionet_dev *dev, char *pkt, size_t sz, int *spc)
{
uint64_t q_gpa;
uint32_t vr_sz;
@@ -1083,7 +1121,7 @@ vionet_enq_rx(struct vionet_dev *dev, ch
ptrdiff_t off;
int ret;
char *vr;
- ssize_t rem;
+ size_t rem;
struct vring_desc *desc, *pkt_desc, *hdr_desc;
struct vring_avail *avail;
struct vring_used *used;
@@ -1092,6 +1130,11 @@ vionet_enq_rx(struct vionet_dev *dev, ch

ret = 0;

+ if (sz < 1 || sz > IP_MAXPACKET + ETHER_HDR_LEN) {
+ log_warn("%s: invalid packet size", __func__);
+ return (0);
+ }
+
if (!(dev->cfg.device_status & VIRTIO_CONFIG_DEVICE_STATUS_DRIVER_OK))
return ret;

@@ -1155,14 +1198,14 @@ vionet_enq_rx(struct vionet_dev *dev, ch

if (rem >= sz) {
if (write_mem(hdr_desc->addr + sizeof(struct virtio_net_hdr),
- pkt, sz)) {
+ pkt, sz)) {
log_warnx("vionet: rx enq packet write_mem error @ "
"0x%llx", pkt_desc->addr);
goto out;
}
} else {
/* Fallback to pkt_desc descriptor */
- if ((uint64_t)pkt_desc->len >= (uint64_t)sz) {
+ if (pkt_desc->len >= sz) {
/* Must be not readable */
if ((pkt_desc->flags & VRING_DESC_F_WRITE) == 0) {
log_warnx("unexpected readable rx desc %d",
@@ -1231,7 +1274,7 @@ vionet_rx(struct vionet_dev *dev)
if (errno != EAGAIN)
log_warn("unexpected read error on vionet "
"device");
- } else if (sz != 0) {
+ } else if (sz > 0) {
eh = (struct ether_header *)buf;
if (!dev->lockedmac || sz < ETHER_HDR_LEN ||
ETHER_IS_MULTICAST(eh->ether_dhost) ||
@@ -1392,8 +1435,8 @@ vionet_notify_tx(struct vionet_dev *dev)
{
uint64_t q_gpa;
uint32_t vr_sz;
- uint16_t idx, pkt_desc_idx, hdr_desc_idx, dxx;
- size_t pktsz;
+ uint16_t idx, pkt_desc_idx, hdr_desc_idx, dxx, cnt;
+ size_t pktsz, chunk_size = 0;
ssize_t dhcpsz;
int ret, num_enq, ofs, spc;
char *vr, *pkt, *dhcppkt;
@@ -1440,10 +1483,23 @@ vionet_notify_tx(struct vionet_dev *dev)
hdr_desc = &desc[hdr_desc_idx];
pktsz = 0;

+ cnt = 0;
dxx = hdr_desc_idx;
do {
pktsz += desc[dxx].len;
dxx = desc[dxx].next;
+
+ /*
+ * Virtio 1.0, cs04, section 2.4.5:
+ * "The number of descriptors in the table is defined
+ * by the queue size for this virtqueue: this is the
+ * maximum possible descriptor chain length."
+ */
+ if (++cnt >= VIONET_QUEUE_SIZE) {
+ log_warnx("%s: descriptor table invalid",
+ __func__);
+ goto out;
+ }
} while (desc[dxx].flags & VRING_DESC_F_NEXT);

pktsz += desc[dxx].len;
@@ -1451,11 +1507,12 @@ vionet_notify_tx(struct vionet_dev *dev)
/* Remove virtio header descriptor len */
pktsz -= hdr_desc->len;

- /*
- * XXX check sanity pktsz
- * XXX too long and > PAGE_SIZE checks
- * (PAGE_SIZE can be relaxed to 16384 later)
- */
+ /* Only allow buffer len < max IP packet + Ethernet header */
+ if (pktsz > IP_MAXPACKET + ETHER_HDR_LEN) {
+ log_warnx("%s: invalid packet size %lu", __func__,
+ pktsz);
+ goto out;
+ }
pkt = malloc(pktsz);
if (pkt == NULL) {
log_warn("malloc error alloc packet buf");
@@ -1474,9 +1531,16 @@ vionet_notify_tx(struct vionet_dev *dev)
goto out;
}

+ /* Check we don't read beyond allocated pktsz */
+ if (pkt_desc->len > pktsz - ofs) {
+ log_warnx("%s: descriptor len past pkt len",
+ __func__);
+ chunk_size = pktsz - ofs - pkt_desc->len;
+ } else
+ chunk_size = pkt_desc->len;
+
/* Read packet from descriptor ring */
- if (read_mem(pkt_desc->addr, pkt + ofs,
- pkt_desc->len)) {
+ if (read_mem(pkt_desc->addr, pkt + ofs, chunk_size)) {
log_warnx("vionet: packet read_mem error "
"@ 0x%llx", pkt_desc->addr);
goto out;
@@ -1494,9 +1558,15 @@ vionet_notify_tx(struct vionet_dev *dev)
goto out;
}

+ /* Check we don't read beyond allocated pktsz */
+ if (pkt_desc->len > pktsz - ofs) {
+ log_warnx("%s: descriptor len past pkt len", __func__);
+ chunk_size = pktsz - ofs - pkt_desc->len;
+ } else
+ chunk_size = pkt_desc->len;
+
/* Read packet from descriptor ring */
- if (read_mem(pkt_desc->addr, pkt + ofs,
- pkt_desc->len)) {
+ if (read_mem(pkt_desc->addr, pkt + ofs, chunk_size)) {
log_warnx("vionet: packet read_mem error @ "
"0x%llx", pkt_desc->addr);
goto out;
Index: usr.sbin/vmd/virtio.h
===================================================================
RCS file: /cvs/src/usr.sbin/vmd/virtio.h,v
diff -u -p -r1.35 virtio.h
--- usr.sbin/vmd/virtio.h 11 Dec 2019 06:45:16 -0000 1.35
+++ usr.sbin/vmd/virtio.h 13 May 2021 18:56:16 -0000
@@ -297,7 +297,7 @@ int vionet_notifyq(struct vionet_dev *);
void vionet_notify_rx(struct vionet_dev *);
int vionet_notify_tx(struct vionet_dev *);
void vionet_process_rx(uint32_t);
-int vionet_enq_rx(struct vionet_dev *, char *, ssize_t, int *);
+int vionet_enq_rx(struct vionet_dev *, char *, size_t, int *);

int vmmci_io(int, uint16_t, uint32_t *, uint8_t *, void *, uint8_t);
int vmmci_dump(int);