vhost_vring_set_num() accepts any non-zero power-of-two queue size that
fits in 16 bits. vhost-vdpa then passes that value to set_vq_num()
without comparing it with get_vq_num_max().
A process with access to /dev/vhost-vdpa-* can therefore configure a
queue larger than the device advertises. With vdpa_sim, the worker can
walk descriptors beyond the mapped descriptor ring. KASAN reports a
16-byte out-of-bounds read, corresponding to one vring_desc, in the
vringh IOTLB path:
BUG: KASAN: out-of-bounds in _copy_from_iter
Read of size 16
copy_from_iotlb
copydesc_iotlb
vringh_getdesc_iotlb
vdpasim_net_work
Cache get_vq_num_max() immediately after reset. Some backends derive
it from writable queue-size state, so querying it after SET_NUM may
return the current size instead of the device capability. Invalidate
the cached value before reset so a failed reset leaves SET_NUM
disabled.
For VHOST_SET_VRING_NUM, copy the complete vring state once and use
the same index and size for validation, vq->num, and set_vq_num().
This ensures that validation and use operate on the same copied values.
Fixes: 4c8cf31885f6 ("vhost: introduce vDPA-based backend")
Signed-off-by: Jia Jia <[email protected]>
---
v3:
- Copy VHOST_SET_VRING_NUM once and use the copied values throughout,
avoiding the double-fetch race in v2.
- Cache the device-wide maximum after reset so mutable queue state
cannot break a later legal resize; fail closed if reset fails.
v2:
- Add the missing Signed-off-by tag.
drivers/vhost/vdpa.c | 44 +++++++++++++++++++++++++++++++++++++-------
1 file changed, 37 insertions(+), 7 deletions(-)
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 692564b1bcbb..f2eedb0464c2 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -53,6 +53,7 @@ struct vhost_vdpa {
struct cdev cdev;
atomic_t opened;
u32 nvqs;
+ u16 vq_num_max;
int virtio_id;
int minor;
struct eventfd_ctx *config_ctx;
@@ -229,7 +230,9 @@ static void vhost_vdpa_unsetup_vq_irq(struct vhost_vdpa *v,
u16 qid)
static int _compat_vdpa_reset(struct vhost_vdpa *v)
{
struct vdpa_device *vdpa = v->vdpa;
+ const struct vdpa_config_ops *ops = vdpa->config;
u32 flags = 0;
+ int ret;
v->suspended = false;
@@ -239,7 +242,14 @@ static int _compat_vdpa_reset(struct vhost_vdpa *v)
VDPA_RESET_F_CLEAN_MAP : 0;
}
- return vdpa_reset(vdpa, flags);
+ v->vq_num_max = 0;
+ ret = vdpa_reset(vdpa, flags);
+ if (!ret) {
+ /* Some backends derive the max from mutable queue state. */
+ v->vq_num_max = ops->get_vq_num_max(vdpa);
+ }
+
+ return ret;
}
static int vhost_vdpa_reset(struct vhost_vdpa *v)
@@ -641,9 +651,15 @@ static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v,
unsigned int cmd,
u32 idx;
long r;
- r = get_user(idx, (u32 __user *)argp);
- if (r < 0)
- return r;
+ if (cmd == VHOST_SET_VRING_NUM) {
+ if (copy_from_user(&s, argp, sizeof(s)))
+ return -EFAULT;
+ idx = s.index;
+ } else {
+ r = get_user(idx, (u32 __user *)argp);
+ if (r < 0)
+ return r;
+ }
if (idx >= v->nvqs)
return -ENOBUFS;
@@ -652,6 +668,23 @@ static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v,
unsigned int cmd,
vq = &v->vqs[idx];
switch (cmd) {
+ case VHOST_SET_VRING_NUM:
+ mutex_lock(&vq->mutex);
+ if (vq->private_data) {
+ r = -EBUSY;
+ } else if (!s.num || s.num > 0xffff ||
+ s.num > v->vq_num_max ||
+ (s.num & (s.num - 1))) {
+ r = -EINVAL;
+ } else {
+ vq->num = s.num;
+ r = 0;
+ }
+ mutex_unlock(&vq->mutex);
+ if (r)
+ return r;
+ ops->set_vq_num(vdpa, idx, s.num);
+ return 0;
case VHOST_VDPA_SET_VRING_ENABLE:
if (copy_from_user(&s, argp, sizeof(s)))
return -EFAULT;
@@ -765,9 +798,6 @@ static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v,
unsigned int cmd,
ops->set_vq_cb(vdpa, idx, &cb);
break;
- case VHOST_SET_VRING_NUM:
- ops->set_vq_num(vdpa, idx, vq->num);
- break;
}
return r;
--
2.34.1
