diff options
| author | Pierre Pronchery <pierre@freebsdfoundation.org> | 2024-08-27 13:57:32 +0000 |
|---|---|---|
| committer | Ed Maste <emaste@FreeBSD.org> | 2024-10-02 20:37:49 +0000 |
| commit | 6eb7879f426129aa38f4e8b0d57ab7456e4eb351 (patch) | |
| tree | 5753aed676a7bbb9c372c5d3106bddc3b0c6df24 | |
| parent | 37bea3b062eaeff8c015d628981ab97cc9c90e56 (diff) | |
bhyve: avoid TOCTOU on iov_len in virtio_vq_recordon()
Avoid a race condition when accessing guest memory, by reading memory
contents only once.
This has also been applied to _vq_record() in
sys/dev/beri/virtio/virtio.c, as per markj@'s suggestion.
Reported by: Synacktiv
Reviewed by: markj
Security: HYP-10
Sponsored by: The Alpha-Omega Project
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D45735
(cherry picked from commit 869d760cb9d7a307faa2fbe8c1c2b238a81b74d4)
(cherry picked from commit ed03c309908687bdb9f71dc6d9c9c8a92c54fc20)
| -rw-r--r-- | sys/dev/beri/virtio/virtio.c | 11 | ||||
| -rw-r--r-- | usr.sbin/bhyve/virtio.c | 9 |
2 files changed, 15 insertions, 5 deletions
diff --git a/sys/dev/beri/virtio/virtio.c b/sys/dev/beri/virtio/virtio.c index c249d1c9d37b..7302bc39d799 100644 --- a/sys/dev/beri/virtio/virtio.c +++ b/sys/dev/beri/virtio/virtio.c @@ -107,12 +107,17 @@ paddr_unmap(void *phys, uint32_t size) static inline void _vq_record(uint32_t offs, int i, volatile struct vring_desc *vd, struct iovec *iov, int n_iov, uint16_t *flags) { + uint32_t len; + uint64_t addr; + if (i >= n_iov) return; - iov[i].iov_base = paddr_map(offs, be64toh(vd->addr), - be32toh(vd->len)); - iov[i].iov_len = be32toh(vd->len); + len = atomic_load_32(&vd->len); + addr = atomic_load_64(&vd->addr); + iov[i].iov_base = paddr_map(offs, be64toh(addr), + be32toh(len)); + iov[i].iov_len = be32toh(len); if (flags != NULL) flags[i] = be16toh(vd->flags); } diff --git a/usr.sbin/bhyve/virtio.c b/usr.sbin/bhyve/virtio.c index 5294da670677..d11f5683e7b7 100644 --- a/usr.sbin/bhyve/virtio.c +++ b/usr.sbin/bhyve/virtio.c @@ -215,10 +215,15 @@ static inline void _vq_record(int i, struct vring_desc *vd, struct vmctx *ctx, struct iovec *iov, int n_iov, struct vi_req *reqp) { + uint32_t len; + uint64_t addr; + if (i >= n_iov) return; - iov[i].iov_base = paddr_guest2host(ctx, vd->addr, vd->len); - iov[i].iov_len = vd->len; + len = atomic_load_32(&vd->len); + addr = atomic_load_64(&vd->addr); + iov[i].iov_len = len; + iov[i].iov_base = paddr_guest2host(ctx, addr, len); if ((vd->flags & VRING_DESC_F_WRITE) == 0) reqp->readable++; else |