diff options
author | Alexander Motin <mav@FreeBSD.org> | 2018-12-07 20:30:00 +0000 |
---|---|---|
committer | Alexander Motin <mav@FreeBSD.org> | 2018-12-07 20:30:00 +0000 |
commit | 99fa47de81c19e6066d0bd8e8732e452a4823de6 (patch) | |
tree | d80945d80b5d2f67fdb8831870120f62b12433da /usr.sbin | |
parent | 9ce46e810768b11d5f6af5e4215593e01599a24b (diff) | |
download | src-99fa47de81c19e6066d0bd8e8732e452a4823de6.tar.gz src-99fa47de81c19e6066d0bd8e8732e452a4823de6.zip |
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
Notes
Notes:
svn path=/head/; revision=341705
Diffstat (limited to 'usr.sbin')
-rw-r--r-- | usr.sbin/bhyve/iov.c | 57 | ||||
-rw-r--r-- | usr.sbin/bhyve/iov.h | 13 | ||||
-rw-r--r-- | usr.sbin/bhyve/pci_virtio_scsi.c | 10 |
3 files changed, 45 insertions, 35 deletions
diff --git a/usr.sbin/bhyve/iov.c b/usr.sbin/bhyve/iov.c index c564bd8ae50f..54ea22aa9498 100644 --- a/usr.sbin/bhyve/iov.c +++ b/usr.sbin/bhyve/iov.c @@ -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); } diff --git a/usr.sbin/bhyve/iov.h b/usr.sbin/bhyve/iov.h index 87fa4c1dcfe6..e3b5916edb10 100644 --- a/usr.sbin/bhyve/iov.h +++ b/usr.sbin/bhyve/iov.h @@ -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_ */ diff --git a/usr.sbin/bhyve/pci_virtio_scsi.c b/usr.sbin/bhyve/pci_virtio_scsi.c index ffa9a85680c2..78e109bc936d 100644 --- a/usr.sbin/bhyve/pci_virtio_scsi.c +++ b/usr.sbin/bhyve/pci_virtio_scsi.c @@ -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 |