Fix several iov handling bugs in bhyve virtio-scsi backend.

- buf_to_iov() does not use buflen parameter, allowing out of bound read.
 - buf_to_iov() leaks memory if seek argument > 0.
 - iov_to_buf() doesn't need to reallocate buffer for every segment.
 - there is no point to use size_t for iov counts, int is more then enough.
 - some iov function arguments can be constified.
 - pci_vtscsi_request_handle() used truncate_iov() incorrectly, allowing
   getting out of buffer and possibly corrupting data.
 - pci_vtscsi_controlq_notify() written returned status at wrong offset.
 - pci_vtscsi_controlq_notify() leaked one buffer per event.

Reported by:	wg
Reviewed by:	araujo
MFC after:	1 week
Sponsored by:	iXsystems, Inc.
Differential Revision:	https://reviews.freebsd.org/D18465
This commit is contained in:
Alexander Motin 2018-12-07 20:30:00 +00:00
parent 9ce46e8107
commit 99fa47de81
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/head/; revision=341705
3 changed files with 45 additions and 35 deletions

View File

@ -2,6 +2,7 @@
* SPDX-License-Identifier: BSD-2-Clause-FreeBSD
*
* Copyright (c) 2016 Jakub Klama <jceel@FreeBSD.org>.
* Copyright (c) 2018 Alexander Motin <mav@FreeBSD.org>
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@ -39,12 +40,12 @@ __FBSDID("$FreeBSD$");
#include "iov.h"
void
seek_iov(struct iovec *iov1, size_t niov1, struct iovec *iov2, size_t *niov2,
seek_iov(const struct iovec *iov1, int niov1, struct iovec *iov2, int *niov2,
size_t seek)
{
size_t remainder = 0;
size_t left = seek;
size_t i, j;
int i, j;
for (i = 0; i < niov1; i++) {
size_t toseek = MIN(left, iov1[i].iov_len);
@ -69,9 +70,10 @@ seek_iov(struct iovec *iov1, size_t niov1, struct iovec *iov2, size_t *niov2,
}
size_t
count_iov(struct iovec *iov, size_t niov)
count_iov(const struct iovec *iov, int niov)
{
size_t i, total = 0;
size_t total = 0;
int i;
for (i = 0; i < niov; i++)
total += iov[i].iov_len;
@ -79,35 +81,36 @@ count_iov(struct iovec *iov, size_t niov)
return (total);
}
size_t
truncate_iov(struct iovec *iov, size_t niov, size_t length)
void
truncate_iov(struct iovec *iov, int *niov, size_t length)
{
size_t i, done = 0;
size_t done = 0;
int i;
for (i = 0; i < niov; i++) {
for (i = 0; i < *niov; i++) {
size_t toseek = MIN(length - done, iov[i].iov_len);
done += toseek;
if (toseek < iov[i].iov_len) {
if (toseek <= iov[i].iov_len) {
iov[i].iov_len = toseek;
return (i + 1);
*niov = i + 1;
return;
}
}
return (niov);
}
ssize_t
iov_to_buf(struct iovec *iov, size_t niov, void **buf)
iov_to_buf(const struct iovec *iov, int niov, void **buf)
{
size_t i, ptr = 0, total = 0;
size_t ptr, total;
int i;
for (i = 0; i < niov; i++) {
total += iov[i].iov_len;
*buf = realloc(*buf, total);
if (*buf == NULL)
return (-1);
total = count_iov(iov, niov);
*buf = realloc(*buf, total);
if (*buf == NULL)
return (-1);
for (i = 0, ptr = 0; i < niov; i++) {
memcpy(*buf + ptr, iov[i].iov_base, iov[i].iov_len);
ptr += iov[i].iov_len;
}
@ -116,12 +119,12 @@ iov_to_buf(struct iovec *iov, size_t niov, void **buf)
}
ssize_t
buf_to_iov(void *buf, size_t buflen, struct iovec *iov, size_t niov,
buf_to_iov(const void *buf, size_t buflen, struct iovec *iov, int niov,
size_t seek)
{
struct iovec *diov;
size_t ndiov, i;
uintptr_t off = 0;
int ndiov, i;
size_t off = 0, len;
if (seek > 0) {
diov = malloc(sizeof(struct iovec) * niov);
@ -131,11 +134,15 @@ buf_to_iov(void *buf, size_t buflen, struct iovec *iov, size_t niov,
ndiov = niov;
}
for (i = 0; i < ndiov; i++) {
memcpy(diov[i].iov_base, buf + off, diov[i].iov_len);
off += diov[i].iov_len;
for (i = 0; i < ndiov && off < buflen; i++) {
len = MIN(diov[i].iov_len, buflen - off);
memcpy(diov[i].iov_base, buf + off, len);
off += len;
}
if (seek > 0)
free(diov);
return ((ssize_t)off);
}

View File

@ -2,6 +2,7 @@
* SPDX-License-Identifier: BSD-2-Clause-FreeBSD
*
* Copyright (c) 2016 Jakub Klama <jceel@FreeBSD.org>.
* Copyright (c) 2018 Alexander Motin <mav@FreeBSD.org>
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@ -32,12 +33,12 @@
#ifndef _IOV_H_
#define _IOV_H_
void seek_iov(struct iovec *iov1, size_t niov1, struct iovec *iov2,
size_t *niov2, size_t seek);
size_t truncate_iov(struct iovec *iov, size_t niov, size_t length);
size_t count_iov(struct iovec *iov, size_t niov);
ssize_t iov_to_buf(struct iovec *iov, size_t niov, void **buf);
ssize_t buf_to_iov(void *buf, size_t buflen, struct iovec *iov, size_t niov,
void seek_iov(const struct iovec *iov1, int niov1, struct iovec *iov2,
int *niov2, size_t seek);
void truncate_iov(struct iovec *iov, int *niov, size_t length);
size_t count_iov(const struct iovec *iov, int niov);
ssize_t iov_to_buf(const struct iovec *iov, int niov, void **buf);
ssize_t buf_to_iov(const void *buf, size_t buflen, struct iovec *iov, int niov,
size_t seek);
#endif /* _IOV_H_ */

View File

@ -462,7 +462,7 @@ pci_vtscsi_request_handle(struct pci_vtscsi_queue *q, struct iovec *iov_in,
struct pci_vtscsi_req_cmd_wr *cmd_wr;
struct iovec data_iov_in[VTSCSI_MAXSEG], data_iov_out[VTSCSI_MAXSEG];
union ctl_io *io;
size_t data_niov_in, data_niov_out;
int data_niov_in, data_niov_out;
void *ext_data_ptr = NULL;
uint32_t ext_data_len = 0, ext_sg_entries = 0;
int err;
@ -472,8 +472,8 @@ pci_vtscsi_request_handle(struct pci_vtscsi_queue *q, struct iovec *iov_in,
seek_iov(iov_out, niov_out, data_iov_out, &data_niov_out,
VTSCSI_OUT_HEADER_LEN(sc));
truncate_iov(iov_in, niov_in, VTSCSI_IN_HEADER_LEN(sc));
truncate_iov(iov_out, niov_out, VTSCSI_OUT_HEADER_LEN(sc));
truncate_iov(iov_in, &niov_in, VTSCSI_IN_HEADER_LEN(sc));
truncate_iov(iov_out, &niov_out, VTSCSI_OUT_HEADER_LEN(sc));
iov_to_buf(iov_in, niov_in, (void **)&cmd_rd);
cmd_wr = malloc(VTSCSI_OUT_HEADER_LEN(sc));
@ -552,7 +552,8 @@ pci_vtscsi_controlq_notify(void *vsc, struct vqueue_info *vq)
n = vq_getchain(vq, &idx, iov, VTSCSI_MAXSEG, NULL);
bufsize = iov_to_buf(iov, n, &buf);
iolen = pci_vtscsi_control_handle(sc, buf, bufsize);
buf_to_iov(buf + bufsize - iolen, iolen, iov, n, iolen);
buf_to_iov(buf + bufsize - iolen, iolen, iov, n,
bufsize - iolen);
/*
* Release this chain and handle more
@ -560,6 +561,7 @@ pci_vtscsi_controlq_notify(void *vsc, struct vqueue_info *vq)
vq_relchain(vq, idx, iolen);
}
vq_endchains(vq, 1); /* Generate interrupt if appropriate. */
free(buf);
}
static void