Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dm: virtio: check for paddr_guest2host return value #5453

Merged
merged 1 commit into from Nov 6, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
dm: virtio: check for paddr_guest2host return value
paddr_guest2host can return NULL, but code paths in virtio
are not checking the return value.
_vq_record() initializes iov_base pointer using paddr_guest2host()
but there is nothing in the flow that checks for NULL.
Chane _vq_record to return -1 in case the address translation
has failed.

Tracked-On: #5452
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Acked-by: Wang, Yu1 <yu1.wang@intel.com>
  • Loading branch information
tomasbw committed Nov 4, 2020
commit ae0ab82434509d6e75f4a2f1e1a0dd2ee3dc3681
37 changes: 32 additions & 5 deletions devicemodel/hw/pci/virtio/virtio.c
Expand Up @@ -296,6 +296,8 @@ virtio_vq_init(struct virtio_base *base, uint32_t pfn)
phys = (uint64_t)pfn << VRING_PAGE_BITS;
size = vring_size(vq->qsize, VIRTIO_PCI_VRING_ALIGN);
vb = paddr_guest2host(base->dev->vmctx, phys, size);
if (!vb)
goto error;

/* First page(s) are descriptors... */
vq->desc = (struct vring_desc *)vb;
Expand All @@ -318,6 +320,12 @@ virtio_vq_init(struct virtio_base *base, uint32_t pfn)
/* Mark queue as allocated after initialization is complete. */
mb();
vq->flags = VQ_ALLOC;

return;

error:
vq->flags = 0;
pr_err("%s: vq enable failed\n", __func__);
}

/*
Expand Down Expand Up @@ -382,17 +390,25 @@ virtio_vq_enable(struct virtio_base *base)
/*
* Helper inline for vq_getchain(): record the i'th "real"
* descriptor.
* Return 0 on success and -1 when i is out of range or mapping
* fails.
*/
static inline void
static inline int
_vq_record(int i, volatile struct vring_desc *vd, struct vmctx *ctx,
struct iovec *iov, int n_iov, uint16_t *flags) {

void *host_addr;

if (i >= n_iov)
return;
iov[i].iov_base = paddr_guest2host(ctx, vd->addr, vd->len);
return -1;
host_addr = paddr_guest2host(ctx, vd->addr, vd->len);
if (!host_addr)
return -1;
iov[i].iov_base = host_addr;
iov[i].iov_len = vd->len;
if (flags != NULL)
flags[i] = vd->flags;
return 0;
}
#define VQ_MAX_DESCRIPTORS 512 /* see below */

Expand Down Expand Up @@ -495,7 +511,10 @@ vq_getchain(struct virtio_vq_info *vq, uint16_t *pidx,
}
vdir = &vq->desc[next];
if ((vdir->flags & VRING_DESC_F_INDIRECT) == 0) {
_vq_record(i, vdir, ctx, iov, n_iov, flags);
if (_vq_record(i, vdir, ctx, iov, n_iov, flags)) {
pr_err("%s: mapping to host failed\r\n", name);
return -1;
}
i++;
} else if ((base->device_caps &
(1 << VIRTIO_RING_F_INDIRECT_DESC)) == 0) {
Expand All @@ -513,6 +532,11 @@ vq_getchain(struct virtio_vq_info *vq, uint16_t *pidx,
}
vindir = paddr_guest2host(ctx,
vdir->addr, vdir->len);

if (!vindir) {
pr_err("%s cannot get host memory\r\n", name);
return -1;
}
/*
* Indirects start at the 0th, then follow
* their own embedded "next"s until those run
Expand All @@ -529,7 +553,10 @@ vq_getchain(struct virtio_vq_info *vq, uint16_t *pidx,
name);
return -1;
}
_vq_record(i, vp, ctx, iov, n_iov, flags);
if (_vq_record(i, vp, ctx, iov, n_iov, flags)) {
pr_err("%s: mapping to host failed\r\n", name);
return -1;
}
if (++i > VQ_MAX_DESCRIPTORS)
goto loopy;
if ((vp->flags & VRING_DESC_F_NEXT) == 0)
Expand Down