Skip to content

Commit d06c942

Browse files
committed
Merge tag 'for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost
Pull vhost,virtio,vdpa bugfixes from Michael Tsirkin: "Misc fixes all over the place. Revert of virtio used length validation series: the approach taken does not seem to work, breaking too many guests in the process. We'll need to do length validation using some other approach" [ This merge also ends up reverting commit f7a36b0 ("vsock/virtio: suppress used length validation"), which came in through the networking tree in the meantime, and was part of that whole used length validation series - Linus ] * tag 'for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost: vdpa_sim: avoid putting an uninitialized iova_domain vhost-vdpa: clean irqs before reseting vdpa device virtio-blk: modify the value type of num in virtio_queue_rq() vhost/vsock: cleanup removing `len` variable vhost/vsock: fix incorrect used length reported to the guest Revert "virtio_ring: validate used buffer length" Revert "virtio-net: don't let virtio core to validate used length" Revert "virtio-blk: don't let virtio core to validate used length" Revert "virtio-scsi: don't let virtio core to validate used buffer length"
2 parents 9557e60 + bb93ce4 commit d06c942

File tree

9 files changed

+9
-76
lines changed

9 files changed

+9
-76
lines changed

drivers/block/virtio_blk.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ static blk_status_t virtio_queue_rq(struct blk_mq_hw_ctx *hctx,
316316
struct request *req = bd->rq;
317317
struct virtblk_req *vbr = blk_mq_rq_to_pdu(req);
318318
unsigned long flags;
319-
unsigned int num;
319+
int num;
320320
int qid = hctx->queue_num;
321321
bool notify = false;
322322
blk_status_t status;
@@ -1049,7 +1049,6 @@ static struct virtio_driver virtio_blk = {
10491049
.feature_table_size = ARRAY_SIZE(features),
10501050
.feature_table_legacy = features_legacy,
10511051
.feature_table_size_legacy = ARRAY_SIZE(features_legacy),
1052-
.suppress_used_validation = true,
10531052
.driver.name = KBUILD_MODNAME,
10541053
.driver.owner = THIS_MODULE,
10551054
.id_table = id_table,

drivers/net/virtio_net.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3423,7 +3423,6 @@ static struct virtio_driver virtio_net_driver = {
34233423
.feature_table_size = ARRAY_SIZE(features),
34243424
.feature_table_legacy = features_legacy,
34253425
.feature_table_size_legacy = ARRAY_SIZE(features_legacy),
3426-
.suppress_used_validation = true,
34273426
.driver.name = KBUILD_MODNAME,
34283427
.driver.owner = THIS_MODULE,
34293428
.id_table = id_table,

drivers/scsi/virtio_scsi.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -977,7 +977,6 @@ static unsigned int features[] = {
977977
static struct virtio_driver virtio_scsi_driver = {
978978
.feature_table = features,
979979
.feature_table_size = ARRAY_SIZE(features),
980-
.suppress_used_validation = true,
981980
.driver.name = KBUILD_MODNAME,
982981
.driver.owner = THIS_MODULE,
983982
.id_table = id_table,

drivers/vdpa/vdpa_sim/vdpa_sim.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -591,8 +591,11 @@ static void vdpasim_free(struct vdpa_device *vdpa)
591591
vringh_kiov_cleanup(&vdpasim->vqs[i].in_iov);
592592
}
593593

594-
put_iova_domain(&vdpasim->iova);
595-
iova_cache_put();
594+
if (vdpa_get_dma_dev(vdpa)) {
595+
put_iova_domain(&vdpasim->iova);
596+
iova_cache_put();
597+
}
598+
596599
kvfree(vdpasim->buffer);
597600
if (vdpasim->iommu)
598601
vhost_iotlb_free(vdpasim->iommu);

drivers/vhost/vdpa.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1014,12 +1014,12 @@ static int vhost_vdpa_release(struct inode *inode, struct file *filep)
10141014

10151015
mutex_lock(&d->mutex);
10161016
filep->private_data = NULL;
1017+
vhost_vdpa_clean_irq(v);
10171018
vhost_vdpa_reset(v);
10181019
vhost_dev_stop(&v->vdev);
10191020
vhost_vdpa_iotlb_free(v);
10201021
vhost_vdpa_free_domain(v);
10211022
vhost_vdpa_config_put(v);
1022-
vhost_vdpa_clean_irq(v);
10231023
vhost_dev_cleanup(&v->vdev);
10241024
kfree(v->vdev.vqs);
10251025
mutex_unlock(&d->mutex);

drivers/vhost/vsock.c

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -511,8 +511,6 @@ static void vhost_vsock_handle_tx_kick(struct vhost_work *work)
511511

512512
vhost_disable_notify(&vsock->dev, vq);
513513
do {
514-
u32 len;
515-
516514
if (!vhost_vsock_more_replies(vsock)) {
517515
/* Stop tx until the device processes already
518516
* pending replies. Leave tx virtqueue
@@ -540,7 +538,7 @@ static void vhost_vsock_handle_tx_kick(struct vhost_work *work)
540538
continue;
541539
}
542540

543-
len = pkt->len;
541+
total_len += sizeof(pkt->hdr) + pkt->len;
544542

545543
/* Deliver to monitoring devices all received packets */
546544
virtio_transport_deliver_tap_pkt(pkt);
@@ -553,9 +551,7 @@ static void vhost_vsock_handle_tx_kick(struct vhost_work *work)
553551
else
554552
virtio_transport_free_pkt(pkt);
555553

556-
len += sizeof(pkt->hdr);
557-
vhost_add_used(vq, head, len);
558-
total_len += len;
554+
vhost_add_used(vq, head, 0);
559555
added = true;
560556
} while(likely(!vhost_exceeds_weight(vq, ++pkts, total_len)));
561557

drivers/virtio/virtio_ring.c

Lines changed: 0 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,6 @@
1414
#include <linux/spinlock.h>
1515
#include <xen/xen.h>
1616

17-
static bool force_used_validation = false;
18-
module_param(force_used_validation, bool, 0444);
19-
2017
#ifdef DEBUG
2118
/* For development, we want to crash whenever the ring is screwed. */
2219
#define BAD_RING(_vq, fmt, args...) \
@@ -185,9 +182,6 @@ struct vring_virtqueue {
185182
} packed;
186183
};
187184

188-
/* Per-descriptor in buffer length */
189-
u32 *buflen;
190-
191185
/* How to notify other side. FIXME: commonalize hcalls! */
192186
bool (*notify)(struct virtqueue *vq);
193187

@@ -496,7 +490,6 @@ static inline int virtqueue_add_split(struct virtqueue *_vq,
496490
unsigned int i, n, avail, descs_used, prev, err_idx;
497491
int head;
498492
bool indirect;
499-
u32 buflen = 0;
500493

501494
START_USE(vq);
502495

@@ -578,7 +571,6 @@ static inline int virtqueue_add_split(struct virtqueue *_vq,
578571
VRING_DESC_F_NEXT |
579572
VRING_DESC_F_WRITE,
580573
indirect);
581-
buflen += sg->length;
582574
}
583575
}
584576
/* Last one doesn't continue. */
@@ -618,10 +610,6 @@ static inline int virtqueue_add_split(struct virtqueue *_vq,
618610
else
619611
vq->split.desc_state[head].indir_desc = ctx;
620612

621-
/* Store in buffer length if necessary */
622-
if (vq->buflen)
623-
vq->buflen[head] = buflen;
624-
625613
/* Put entry in available array (but don't update avail->idx until they
626614
* do sync). */
627615
avail = vq->split.avail_idx_shadow & (vq->split.vring.num - 1);
@@ -796,11 +784,6 @@ static void *virtqueue_get_buf_ctx_split(struct virtqueue *_vq,
796784
BAD_RING(vq, "id %u is not a head!\n", i);
797785
return NULL;
798786
}
799-
if (vq->buflen && unlikely(*len > vq->buflen[i])) {
800-
BAD_RING(vq, "used len %d is larger than in buflen %u\n",
801-
*len, vq->buflen[i]);
802-
return NULL;
803-
}
804787

805788
/* detach_buf_split clears data, so grab it now. */
806789
ret = vq->split.desc_state[i].data;
@@ -1079,7 +1062,6 @@ static int virtqueue_add_indirect_packed(struct vring_virtqueue *vq,
10791062
unsigned int i, n, err_idx;
10801063
u16 head, id;
10811064
dma_addr_t addr;
1082-
u32 buflen = 0;
10831065

10841066
head = vq->packed.next_avail_idx;
10851067
desc = alloc_indirect_packed(total_sg, gfp);
@@ -1109,8 +1091,6 @@ static int virtqueue_add_indirect_packed(struct vring_virtqueue *vq,
11091091
desc[i].addr = cpu_to_le64(addr);
11101092
desc[i].len = cpu_to_le32(sg->length);
11111093
i++;
1112-
if (n >= out_sgs)
1113-
buflen += sg->length;
11141094
}
11151095
}
11161096

@@ -1164,10 +1144,6 @@ static int virtqueue_add_indirect_packed(struct vring_virtqueue *vq,
11641144
vq->packed.desc_state[id].indir_desc = desc;
11651145
vq->packed.desc_state[id].last = id;
11661146

1167-
/* Store in buffer length if necessary */
1168-
if (vq->buflen)
1169-
vq->buflen[id] = buflen;
1170-
11711147
vq->num_added += 1;
11721148

11731149
pr_debug("Added buffer head %i to %p\n", head, vq);
@@ -1203,7 +1179,6 @@ static inline int virtqueue_add_packed(struct virtqueue *_vq,
12031179
__le16 head_flags, flags;
12041180
u16 head, id, prev, curr, avail_used_flags;
12051181
int err;
1206-
u32 buflen = 0;
12071182

12081183
START_USE(vq);
12091184

@@ -1283,8 +1258,6 @@ static inline int virtqueue_add_packed(struct virtqueue *_vq,
12831258
1 << VRING_PACKED_DESC_F_AVAIL |
12841259
1 << VRING_PACKED_DESC_F_USED;
12851260
}
1286-
if (n >= out_sgs)
1287-
buflen += sg->length;
12881261
}
12891262
}
12901263

@@ -1304,10 +1277,6 @@ static inline int virtqueue_add_packed(struct virtqueue *_vq,
13041277
vq->packed.desc_state[id].indir_desc = ctx;
13051278
vq->packed.desc_state[id].last = prev;
13061279

1307-
/* Store in buffer length if necessary */
1308-
if (vq->buflen)
1309-
vq->buflen[id] = buflen;
1310-
13111280
/*
13121281
* A driver MUST NOT make the first descriptor in the list
13131282
* available before all subsequent descriptors comprising
@@ -1494,11 +1463,6 @@ static void *virtqueue_get_buf_ctx_packed(struct virtqueue *_vq,
14941463
BAD_RING(vq, "id %u is not a head!\n", id);
14951464
return NULL;
14961465
}
1497-
if (vq->buflen && unlikely(*len > vq->buflen[id])) {
1498-
BAD_RING(vq, "used len %d is larger than in buflen %u\n",
1499-
*len, vq->buflen[id]);
1500-
return NULL;
1501-
}
15021466

15031467
/* detach_buf_packed clears data, so grab it now. */
15041468
ret = vq->packed.desc_state[id].data;
@@ -1704,7 +1668,6 @@ static struct virtqueue *vring_create_virtqueue_packed(
17041668
struct vring_virtqueue *vq;
17051669
struct vring_packed_desc *ring;
17061670
struct vring_packed_desc_event *driver, *device;
1707-
struct virtio_driver *drv = drv_to_virtio(vdev->dev.driver);
17081671
dma_addr_t ring_dma_addr, driver_event_dma_addr, device_event_dma_addr;
17091672
size_t ring_size_in_bytes, event_size_in_bytes;
17101673

@@ -1794,15 +1757,6 @@ static struct virtqueue *vring_create_virtqueue_packed(
17941757
if (!vq->packed.desc_extra)
17951758
goto err_desc_extra;
17961759

1797-
if (!drv->suppress_used_validation || force_used_validation) {
1798-
vq->buflen = kmalloc_array(num, sizeof(*vq->buflen),
1799-
GFP_KERNEL);
1800-
if (!vq->buflen)
1801-
goto err_buflen;
1802-
} else {
1803-
vq->buflen = NULL;
1804-
}
1805-
18061760
/* No callback? Tell other side not to bother us. */
18071761
if (!callback) {
18081762
vq->packed.event_flags_shadow = VRING_PACKED_EVENT_FLAG_DISABLE;
@@ -1815,8 +1769,6 @@ static struct virtqueue *vring_create_virtqueue_packed(
18151769
spin_unlock(&vdev->vqs_list_lock);
18161770
return &vq->vq;
18171771

1818-
err_buflen:
1819-
kfree(vq->packed.desc_extra);
18201772
err_desc_extra:
18211773
kfree(vq->packed.desc_state);
18221774
err_desc_state:
@@ -2224,7 +2176,6 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index,
22242176
void (*callback)(struct virtqueue *),
22252177
const char *name)
22262178
{
2227-
struct virtio_driver *drv = drv_to_virtio(vdev->dev.driver);
22282179
struct vring_virtqueue *vq;
22292180

22302181
if (virtio_has_feature(vdev, VIRTIO_F_RING_PACKED))
@@ -2284,15 +2235,6 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index,
22842235
if (!vq->split.desc_extra)
22852236
goto err_extra;
22862237

2287-
if (!drv->suppress_used_validation || force_used_validation) {
2288-
vq->buflen = kmalloc_array(vring.num, sizeof(*vq->buflen),
2289-
GFP_KERNEL);
2290-
if (!vq->buflen)
2291-
goto err_buflen;
2292-
} else {
2293-
vq->buflen = NULL;
2294-
}
2295-
22962238
/* Put everything in free lists. */
22972239
vq->free_head = 0;
22982240
memset(vq->split.desc_state, 0, vring.num *
@@ -2303,8 +2245,6 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index,
23032245
spin_unlock(&vdev->vqs_list_lock);
23042246
return &vq->vq;
23052247

2306-
err_buflen:
2307-
kfree(vq->split.desc_extra);
23082248
err_extra:
23092249
kfree(vq->split.desc_state);
23102250
err_state:

include/linux/virtio.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,6 @@ size_t virtio_max_dma_size(struct virtio_device *vdev);
152152
* @feature_table_size: number of entries in the feature table array.
153153
* @feature_table_legacy: same as feature_table but when working in legacy mode.
154154
* @feature_table_size_legacy: number of entries in feature table legacy array.
155-
* @suppress_used_validation: set to not have core validate used length
156155
* @probe: the function to call when a device is found. Returns 0 or -errno.
157156
* @scan: optional function to call after successful probe; intended
158157
* for virtio-scsi to invoke a scan.
@@ -169,7 +168,6 @@ struct virtio_driver {
169168
unsigned int feature_table_size;
170169
const unsigned int *feature_table_legacy;
171170
unsigned int feature_table_size_legacy;
172-
bool suppress_used_validation;
173171
int (*validate)(struct virtio_device *dev);
174172
int (*probe)(struct virtio_device *dev);
175173
void (*scan)(struct virtio_device *dev);

net/vmw_vsock/virtio_transport.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -731,7 +731,6 @@ static unsigned int features[] = {
731731
static struct virtio_driver virtio_vsock_driver = {
732732
.feature_table = features,
733733
.feature_table_size = ARRAY_SIZE(features),
734-
.suppress_used_validation = true,
735734
.driver.name = KBUILD_MODNAME,
736735
.driver.owner = THIS_MODULE,
737736
.id_table = id_table,

0 commit comments

Comments
 (0)