Re: [PATCH net] vhost_net: fix possible infinite loop
On 2019/5/13 上午1:10, Michael S. Tsirkin wrote:
On Sun, May 05, 2019 at 12:20:24PM +0800, Jason Wang wrote:
On 2019/4/26 下午3:35, Jason Wang wrote:
On 2019/4/26 上午1:52, Michael S. Tsirkin wrote:
On Thu, Apr 25, 2019 at 03:33:19AM -0400, Jason Wang wrote:
When the rx buffer is too small for a packet, we will discard the vq
descriptor and retry it for the next packet:
while ((sock_len = vhost_net_rx_peek_head_len(net, sock->sk,
_intr))) {
...
/* On overrun, truncate and discard */
if (unlikely(headcount > UIO_MAXIOV)) {
iov_iter_init(_iter, READ, vq->iov, 1, 1);
err = sock->ops->recvmsg(sock, ,
1, MSG_DONTWAIT | MSG_TRUNC);
pr_debug("Discarded rx packet: len %zd\n", sock_len);
continue;
}
...
}
This makes it possible to trigger a infinite while..continue loop
through the co-opreation of two VMs like:
1) Malicious VM1 allocate 1 byte rx buffer and try to slow down the
vhost process as much as possible e.g using indirect descriptors or
other.
2) Malicious VM2 generate packets to VM1 as fast as possible
Fixing this by checking against weight at the end of RX and TX
loop. This also eliminate other similar cases when:
- userspace is consuming the packets in the meanwhile
- theoretical TOCTOU attack if guest moving avail index back and forth
to hit the continue after vhost find guest just add new buffers
This addresses CVE-2019-3900.
Fixes: d8316f3991d20 ("vhost: fix total length when packets are
too short")
I agree this is the real issue.
Fixes: 3a4d5c94e9593 ("vhost_net: a kernel-level virtio server")
This is just a red herring imho. We can stick this on any vhost patch :)
Signed-off-by: Jason Wang
---
drivers/vhost/net.c | 41 +
1 file changed, 21 insertions(+), 20 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index df51a35..fb46e6b 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -778,8 +778,9 @@ static void handle_tx_copy(struct vhost_net
*net, struct socket *sock)
int err;
int sent_pkts = 0;
bool sock_can_batch = (sock->sk->sk_sndbuf == INT_MAX);
+ bool next_round = false;
- for (;;) {
+ do {
bool busyloop_intr = false;
if (nvq->done_idx == VHOST_NET_BATCH)
@@ -845,11 +846,10 @@ static void handle_tx_copy(struct
vhost_net *net, struct socket *sock)
vq->heads[nvq->done_idx].id = cpu_to_vhost32(vq, head);
vq->heads[nvq->done_idx].len = 0;
++nvq->done_idx;
- if (vhost_exceeds_weight(++sent_pkts, total_len)) {
- vhost_poll_queue(>poll);
- break;
- }
- }
+ } while (!(next_round = vhost_exceeds_weight(++sent_pkts,
total_len)));
+
+ if (next_round)
+ vhost_poll_queue(>poll);
vhost_tx_batch(net, nvq, sock, );
}
@@ -873,8 +873,9 @@ static void handle_tx_zerocopy(struct
vhost_net *net, struct socket *sock)
struct vhost_net_ubuf_ref *uninitialized_var(ubufs);
bool zcopy_used;
int sent_pkts = 0;
+ bool next_round = false;
- for (;;) {
+ do {
bool busyloop_intr;
/* Release DMAs done buffers first */
@@ -951,11 +952,10 @@ static void handle_tx_zerocopy(struct
vhost_net *net, struct socket *sock)
else
vhost_zerocopy_signal_used(net, vq);
vhost_net_tx_packet(net);
- if (unlikely(vhost_exceeds_weight(++sent_pkts, total_len))) {
- vhost_poll_queue(>poll);
- break;
- }
- }
+ } while (!(next_round = vhost_exceeds_weight(++sent_pkts,
total_len)));
+
+ if (next_round)
+ vhost_poll_queue(>poll);
}
/* Expects to be always run from workqueue - which acts as
@@ -1134,6 +1134,7 @@ static void handle_rx(struct vhost_net *net)
struct iov_iter fixup;
__virtio16 num_buffers;
int recv_pkts = 0;
+ bool next_round = false;
mutex_lock_nested(>mutex, VHOST_NET_VQ_RX);
sock = vq->private_data;
@@ -1153,8 +1154,11 @@ static void handle_rx(struct vhost_net *net)
vq->log : NULL;
mergeable = vhost_has_feature(vq, VIRTIO_NET_F_MRG_RXBUF);
- while ((sock_len = vhost_net_rx_peek_head_len(net, sock->sk,
- _intr))) {
+ do {
+ sock_len = vhost_net_rx_peek_head_len(net, sock->sk,
+ _intr);
+ if (!sock_len)
+ break;
sock_len += sock_hlen;
vhost_len = sock_len + vhost_hlen;
headcount = get_rx_bufs(vq, vq->heads + nvq->done_idx,
@@ -1239,12 +1243,9 @@ static void handle_rx(struct vhost_net *net)
vhost_log_write(vq, vq_log, log, vhost_len,
vq->iov, in);
total_len += vhost_len;
- if (unlikely(vhost_exceeds_weight(++recv_pkts, total_len))) {
- vhost_poll_queue(>poll);
- goto out;
- }
-
[PATCH net] vhost: don't use kmap() to log dirty pages
Vhost log dirty pages directly to a userspace bitmap through GUP and
kmap_atomic() since kernel doesn't have a set_bit_to_user()
helper. This will cause issues for the arch that has virtually tagged
caches. The way to fix is to keep using userspace virtual
address. Fortunately, futex has arch_futex_atomic_op_inuser() which
could be used for setting a bit to user.
Note there're several cases that futex helper can fail e.g a page
fault or the arch that doesn't have the support. For those cases, a
simplified get_user()/put_user() pair protected by a global mutex is
provided as a fallback. The fallback may lead false positive that
userspace may see more dirty pages.
Cc: Christoph Hellwig
Cc: James Bottomley
Cc: Andrea Arcangeli
Cc: Thomas Gleixner
Cc: Ingo Molnar
Cc: Peter Zijlstra
Cc: Darren Hart
Fixes: 3a4d5c94e9593 ("vhost_net: a kernel-level virtio server")
Signed-off-by: Jason Wang
---
Changes from RFC V2:
- drop GUP and provide get_user()/put_user() fallbacks
- round down log_base
Changes from RFC V1:
- switch to use arch_futex_atomic_op_inuser()
---
drivers/vhost/vhost.c | 54 ---
1 file changed, 30 insertions(+), 24 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 351af88..7fa05ba 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -31,6 +31,7 @@
#include
#include
#include
+#include
#include "vhost.h"
@@ -43,6 +44,8 @@
MODULE_PARM_DESC(max_iotlb_entries,
"Maximum number of iotlb entries. (default: 2048)");
+static DEFINE_MUTEX(vhost_log_lock);
+
enum {
VHOST_MEMORY_F_LOG = 0x1,
};
@@ -1692,28 +1695,31 @@ long vhost_dev_ioctl(struct vhost_dev *d, unsigned int
ioctl, void __user *argp)
}
EXPORT_SYMBOL_GPL(vhost_dev_ioctl);
-/* TODO: This is really inefficient. We need something like get_user()
- * (instruction directly accesses the data, with an exception table entry
- * returning -EFAULT). See Documentation/x86/exception-tables.txt.
- */
-static int set_bit_to_user(int nr, void __user *addr)
+static int set_bit_to_user(int nr, u32 __user *addr)
{
- unsigned long log = (unsigned long)addr;
- struct page *page;
- void *base;
- int bit = nr + (log % PAGE_SIZE) * 8;
+ u32 old;
int r;
- r = get_user_pages_fast(log, 1, 1, );
- if (r < 0)
- return r;
- BUG_ON(r != 1);
- base = kmap_atomic(page);
- set_bit(bit, base);
- kunmap_atomic(base);
- set_page_dirty_lock(page);
- put_page(page);
+ r = arch_futex_atomic_op_inuser(FUTEX_OP_OR, 1 << nr, , addr);
+ if (r) {
+ /* Fallback through get_user()/put_user(), this may
+* lead false positive that userspace may see more
+* dirty pages. A mutex is used to synchronize log
+* access between vhost threads.
+*/
+ mutex_lock(_log_lock);
+ r = get_user(old, addr);
+ if (r)
+ goto err;
+ r = put_user(old | 1 << nr, addr);
+ if (r)
+ goto err;
+ mutex_unlock(_log_lock);
+ }
return 0;
+err:
+ mutex_unlock(_log_lock);
+ return r;
}
static int log_write(void __user *log_base,
@@ -1725,13 +1731,13 @@ static int log_write(void __user *log_base,
if (!write_length)
return 0;
write_length += write_address % VHOST_PAGE_SIZE;
+ log_base = (void __user *)((u64)log_base & ~0x3ULL);
+ write_page += ((u64)log_base & 0x3ULL) * 8;
for (;;) {
- u64 base = (u64)(unsigned long)log_base;
- u64 log = base + write_page / 8;
- int bit = write_page % 8;
- if ((u64)(unsigned long)log != log)
- return -EFAULT;
- r = set_bit_to_user(bit, (void __user *)(unsigned long)log);
+ u32 __user *log = (u32 __user *)log_base + write_page / 32;
+ int bit = write_page % 32;
+
+ r = set_bit_to_user(bit, log);
if (r < 0)
return r;
if (write_length <= VHOST_PAGE_SIZE)
--
1.8.3.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH V2] vhost: don't use kmap() to log dirty pages
On 2019/5/10 下午12:48, Jason Wang wrote: On 2019/5/10 上午10:59, Jason Wang wrote: r = get_user_pages_fast(log, 1, 1, ); OK so the trick is that page is pinned so you don't expect arch_futex_atomic_op_inuser below to fail. get_user_pages_fast guarantees page is not going away but does it guarantee PTE won't be invaidated or write protected? Good point, then I think we probably need to do manual fixup through fixup_user_fault() if arch_futex_atomic_op_in_user() fail. This looks like a overkill, we don't need to atomic environment here actually. Instead, just keep pagefault enabled should work. So just introduce arch_futex_atomic_op_inuser_inatomic() variant with pagefault disabled there just for futex should be sufficient. Thanks Ok, instead of using tricks, I think we can gracefully fallback to a get_user()/put_user() pair protected by a mutex. Let me post a non-rfc version for this. Thanks ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [Qemu-devel] [PATCH v8 2/6] virtio-pmem: Add virtio pmem driver
> > Guest reads the persistent memory range information from
> > Qemu over VIRTIO and registers it on nvdimm_bus. It also
> > creates a nd_region object with the persistent memory
> > range information so that existing 'nvdimm/pmem' driver
> > can reserve this into system memory map. This way
> > 'virtio-pmem' driver uses existing functionality of pmem
> > driver to register persistent memory compatible for DAX
> > capable filesystems.
> >
> > This also provides function to perform guest flush over
> > VIRTIO from 'pmem' driver when userspace performs flush
> > on DAX memory range.
> >
> > Signed-off-by: Pankaj Gupta
> > Reviewed-by: Yuval Shaia
>
> Acked-by: Michael S. Tsirkin
Thank you, Michael.
Best regards,
Pankaj
>
> > ---
> > drivers/nvdimm/Makefile | 1 +
> > drivers/nvdimm/nd_virtio.c | 129 +++
> > drivers/nvdimm/virtio_pmem.c | 117
> > drivers/virtio/Kconfig | 10 +++
> > include/linux/virtio_pmem.h | 60 ++
> > include/uapi/linux/virtio_ids.h | 1 +
> > include/uapi/linux/virtio_pmem.h | 10 +++
> > 7 files changed, 328 insertions(+)
> > create mode 100644 drivers/nvdimm/nd_virtio.c
> > create mode 100644 drivers/nvdimm/virtio_pmem.c
> > create mode 100644 include/linux/virtio_pmem.h
> > create mode 100644 include/uapi/linux/virtio_pmem.h
> >
> > diff --git a/drivers/nvdimm/Makefile b/drivers/nvdimm/Makefile
> > index 6f2a088afad6..cefe233e0b52 100644
> > --- a/drivers/nvdimm/Makefile
> > +++ b/drivers/nvdimm/Makefile
> > @@ -5,6 +5,7 @@ obj-$(CONFIG_ND_BTT) += nd_btt.o
> > obj-$(CONFIG_ND_BLK) += nd_blk.o
> > obj-$(CONFIG_X86_PMEM_LEGACY) += nd_e820.o
> > obj-$(CONFIG_OF_PMEM) += of_pmem.o
> > +obj-$(CONFIG_VIRTIO_PMEM) += virtio_pmem.o nd_virtio.o
> >
> > nd_pmem-y := pmem.o
> >
> > diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
> > new file mode 100644
> > index ..ed7ddcc5a62c
> > --- /dev/null
> > +++ b/drivers/nvdimm/nd_virtio.c
> > @@ -0,0 +1,129 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * virtio_pmem.c: Virtio pmem Driver
> > + *
> > + * Discovers persistent memory range information
> > + * from host and provides a virtio based flushing
> > + * interface.
> > + */
> > +#include
> > +#include "nd.h"
> > +
> > + /* The interrupt handler */
> > +void host_ack(struct virtqueue *vq)
> > +{
> > + unsigned int len;
> > + unsigned long flags;
> > + struct virtio_pmem_request *req, *req_buf;
> > + struct virtio_pmem *vpmem = vq->vdev->priv;
> > +
> > + spin_lock_irqsave(>pmem_lock, flags);
> > + while ((req = virtqueue_get_buf(vq, )) != NULL) {
> > + req->done = true;
> > + wake_up(>host_acked);
> > +
> > + if (!list_empty(>req_list)) {
> > + req_buf = list_first_entry(>req_list,
> > + struct virtio_pmem_request, list);
> > + req_buf->wq_buf_avail = true;
> > + wake_up(_buf->wq_buf);
> > + list_del(_buf->list);
> > + }
> > + }
> > + spin_unlock_irqrestore(>pmem_lock, flags);
> > +}
> > +EXPORT_SYMBOL_GPL(host_ack);
> > +
> > + /* The request submission function */
> > +int virtio_pmem_flush(struct nd_region *nd_region)
> > +{
> > + int err, err1;
> > + unsigned long flags;
> > + struct scatterlist *sgs[2], sg, ret;
> > + struct virtio_device *vdev = nd_region->provider_data;
> > + struct virtio_pmem *vpmem = vdev->priv;
> > + struct virtio_pmem_request *req;
> > +
> > + might_sleep();
> > + req = kmalloc(sizeof(*req), GFP_KERNEL);
> > + if (!req)
> > + return -ENOMEM;
> > +
> > + req->done = false;
> > + strcpy(req->name, "FLUSH");
> > + init_waitqueue_head(>host_acked);
> > + init_waitqueue_head(>wq_buf);
> > + INIT_LIST_HEAD(>list);
> > + sg_init_one(, req->name, strlen(req->name));
> > + sgs[0] =
> > + sg_init_one(, >ret, sizeof(req->ret));
> > + sgs[1] =
> > +
> > + spin_lock_irqsave(>pmem_lock, flags);
> > +/*
> > + * If virtqueue_add_sgs returns -ENOSPC then req_vq virtual
> > + * queue does not have free descriptor. We add the request
> > + * to req_list and wait for host_ack to wake us up when free
> > + * slots are available.
> > + */
> > + while ((err = virtqueue_add_sgs(vpmem->req_vq, sgs, 1, 1, req,
> > + GFP_ATOMIC)) == -ENOSPC) {
> > +
> > + dev_err(>dev, "failed to send command to virtio pmem"\
> > + "device, no free slots in the virtqueue\n");
> > + req->wq_buf_avail = false;
> > + list_add_tail(>list, >req_list);
> > + spin_unlock_irqrestore(>pmem_lock, flags);
> > +
> > + /* When host has read buffer, this completes via host_ack */
> > + wait_event(req->wq_buf, req->wq_buf_avail);
> > + spin_lock_irqsave(>pmem_lock, flags);
> > + }
> > +
Re: [PATCH 05/10] s390/cio: introduce DMA pools to cio
On Fri, 10 May 2019 16:10:13 +0200
Cornelia Huck wrote:
> On Fri, 10 May 2019 00:11:12 +0200
> Halil Pasic wrote:
>
> > On Thu, 9 May 2019 12:11:06 +0200
> > Cornelia Huck wrote:
> >
> > > On Wed, 8 May 2019 23:22:10 +0200
> > > Halil Pasic wrote:
> > >
> > > > On Wed, 8 May 2019 15:18:10 +0200 (CEST)
> > > > Sebastian Ott wrote:
> > >
> > > > > > @@ -1063,6 +1163,7 @@ static int __init css_bus_init(void)
> > > > > > unregister_reboot_notifier(_reboot_notifier);
> > > > > > goto out_unregister;
> > > > > > }
> > > > > > + cio_dma_pool_init();
> > > > >
> > > > > This is too late for early devices (ccw console!).
> > > >
> > > > You have already raised concern about this last time (thanks). I think,
> > > > I've addressed this issue: the cio_dma_pool is only used by the airq
> > > > stuff. I don't think the ccw console needs it. Please have an other look
> > > > at patch #6, and explain your concern in more detail if it persists.
> > >
> > > What about changing the naming/adding comments here, so that (1) folks
> > > aren't confused by the same thing in the future and (2) folks don't try
> > > to use that pool for something needed for the early ccw consoles?
> > >
> >
> > I'm all for clarity! Suggestions for better names?
>
> css_aiv_dma_pool, maybe? Or is there other cross-device stuff that may
> need it?
>
Ouch! I was considering to use cio_dma_zalloc() for the adapter
interruption vectors but I ended up between the two chairs in the end.
So with this series there are no uses for cio_dma pool.
I don't feel strongly about this going one way the other.
Against getting rid of the cio_dma_pool and sticking with the speaks
dma_alloc_coherent() that we waste a DMA page per vector, which is a
non obvious side effect.
What speaks against cio_dma_pool is that it is slightly more code, and
this currently can not be used for very early stuff, which I don't
think is relevant. What also used to speak against it is that
allocations asking for more than a page would just fail, but I addressed
that in the patch I've hacked up on top of the series, and I'm going to
paste below. While at it I addressed some other issues as well.
I've also got code that deals with AIRQ_IV_CACHELINE by turning the
kmem_cache into a dma_pool.
Cornelia, Sebastian which approach do you prefer:
1) get rid of cio_dma_pool and AIRQ_IV_CACHELINE, and waste a page per
vector, or
2) go with the approach taken by the patch below?
Regards,
Halil
---8<-
From: Halil Pasic
Date: Sun, 12 May 2019 18:08:05 +0200
Subject: [PATCH] WIP: use cio dma pool for airqs
Let's not waste a DMA page per adapter interrupt bit vector.
---
Lightly tested...
---
arch/s390/include/asm/airq.h | 1 -
drivers/s390/cio/airq.c | 10 +++---
drivers/s390/cio/css.c | 18 +++---
3 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/arch/s390/include/asm/airq.h b/arch/s390/include/asm/airq.h
index 1492d48..981a3eb 100644
--- a/arch/s390/include/asm/airq.h
+++ b/arch/s390/include/asm/airq.h
@@ -30,7 +30,6 @@ void unregister_adapter_interrupt(struct airq_struct *airq);
/* Adapter interrupt bit vector */
struct airq_iv {
unsigned long *vector; /* Adapter interrupt bit vector */
- dma_addr_t vector_dma; /* Adapter interrupt bit vector dma */
unsigned long *avail; /* Allocation bit mask for the bit vector */
unsigned long *bitlock; /* Lock bit mask for the bit vector */
unsigned long *ptr; /* Pointer associated with each bit */
diff --git a/drivers/s390/cio/airq.c b/drivers/s390/cio/airq.c
index 7a5c0a0..f11f437 100644
--- a/drivers/s390/cio/airq.c
+++ b/drivers/s390/cio/airq.c
@@ -136,8 +136,7 @@ struct airq_iv *airq_iv_create(unsigned long bits, unsigned
long flags)
goto out;
iv->bits = bits;
size = iv_size(bits);
- iv->vector = dma_alloc_coherent(cio_get_dma_css_dev(), size,
->vector_dma, GFP_KERNEL);
+ iv->vector = cio_dma_zalloc(size);
if (!iv->vector)
goto out_free;
if (flags & AIRQ_IV_ALLOC) {
@@ -172,8 +171,7 @@ struct airq_iv *airq_iv_create(unsigned long bits, unsigned
long flags)
kfree(iv->ptr);
kfree(iv->bitlock);
kfree(iv->avail);
- dma_free_coherent(cio_get_dma_css_dev(), size, iv->vector,
- iv->vector_dma);
+ cio_dma_free(iv->vector, size);
kfree(iv);
out:
return NULL;
@@ -189,9 +187,7 @@ void airq_iv_release(struct airq_iv *iv)
kfree(iv->data);
kfree(iv->ptr);
kfree(iv->bitlock);
- kfree(iv->vector);
- dma_free_coherent(cio_get_dma_css_dev(), iv_size(iv->bits),
- iv->vector, iv->vector_dma);
+ cio_dma_free(iv->vector, iv_size(iv->bits));
kfree(iv->avail);
kfree(iv);
}
Re: [PATCH v7 0/7] Add virtio-iommu driver
On Tue, Jan 15, 2019 at 12:19:52PM +, Jean-Philippe Brucker wrote: > Implement the virtio-iommu driver, following specification v0.9 [1]. > > This is a simple rebase onto Linux v5.0-rc2. We now use the > dev_iommu_fwspec_get() helper introduced in v5.0 instead of accessing > dev->iommu_fwspec, but there aren't any functional change from v6 [2]. > > Our current goal for virtio-iommu is to get a paravirtual IOMMU working > on Arm, and enable device assignment to guest userspace. In this > use-case the mappings are static, and don't require optimal performance, > so this series tries to keep things simple. However there is plenty more > to do for features and optimizations, and having this base in v5.1 would > be good. Given that most of the changes are to drivers/iommu, I believe > the driver and future changes should go via the IOMMU tree. > > You can find Linux driver and kvmtool device on v0.9.2 branches [3], > module and x86 support on virtio-iommu/devel. Also tested with Eric's > QEMU device [4]. Please note that the series depends on Robin's > probe-deferral fix [5], which will hopefully land in v5.0. > > [1] Virtio-iommu specification v0.9, sources and pdf > git://linux-arm.org/virtio-iommu.git virtio-iommu/v0.9 > http://jpbrucker.net/virtio-iommu/spec/v0.9/virtio-iommu-v0.9.pdf > > [2] [PATCH v6 0/7] Add virtio-iommu driver > > https://lists.linuxfoundation.org/pipermail/iommu/2018-December/032127.html > > [3] git://linux-arm.org/linux-jpb.git virtio-iommu/v0.9.2 > git://linux-arm.org/kvmtool-jpb.git virtio-iommu/v0.9.2 > > [4] [RFC v9 00/17] VIRTIO-IOMMU device > https://www.mail-archive.com/qemu-devel@nongnu.org/msg575578.html > > [5] [PATCH] iommu/of: Fix probe-deferral > https://www.spinics.net/lists/arm-kernel/msg698371.html For virtio things: Acked-by: Michael S. Tsirkin > Jean-Philippe Brucker (7): > dt-bindings: virtio-mmio: Add IOMMU description > dt-bindings: virtio: Add virtio-pci-iommu node > of: Allow the iommu-map property to omit untranslated devices > PCI: OF: Initialize dev->fwnode appropriately > iommu: Add virtio-iommu driver > iommu/virtio: Add probe request > iommu/virtio: Add event queue > > .../devicetree/bindings/virtio/iommu.txt | 66 + > .../devicetree/bindings/virtio/mmio.txt | 30 + > MAINTAINERS |7 + > drivers/iommu/Kconfig | 11 + > drivers/iommu/Makefile|1 + > drivers/iommu/virtio-iommu.c | 1158 + > drivers/of/base.c | 10 +- > drivers/pci/of.c |7 + > include/uapi/linux/virtio_ids.h |1 + > include/uapi/linux/virtio_iommu.h | 161 +++ > 10 files changed, 1449 insertions(+), 3 deletions(-) > create mode 100644 Documentation/devicetree/bindings/virtio/iommu.txt > create mode 100644 drivers/iommu/virtio-iommu.c > create mode 100644 include/uapi/linux/virtio_iommu.h > > -- > 2.19.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH net] vhost_net: fix possible infinite loop
On Sun, May 05, 2019 at 12:20:24PM +0800, Jason Wang wrote:
>
> On 2019/4/26 下午3:35, Jason Wang wrote:
> >
> > On 2019/4/26 上午1:52, Michael S. Tsirkin wrote:
> > > On Thu, Apr 25, 2019 at 03:33:19AM -0400, Jason Wang wrote:
> > > > When the rx buffer is too small for a packet, we will discard the vq
> > > > descriptor and retry it for the next packet:
> > > >
> > > > while ((sock_len = vhost_net_rx_peek_head_len(net, sock->sk,
> > > > _intr))) {
> > > > ...
> > > > /* On overrun, truncate and discard */
> > > > if (unlikely(headcount > UIO_MAXIOV)) {
> > > > iov_iter_init(_iter, READ, vq->iov, 1, 1);
> > > > err = sock->ops->recvmsg(sock, ,
> > > > 1, MSG_DONTWAIT | MSG_TRUNC);
> > > > pr_debug("Discarded rx packet: len %zd\n", sock_len);
> > > > continue;
> > > > }
> > > > ...
> > > > }
> > > >
> > > > This makes it possible to trigger a infinite while..continue loop
> > > > through the co-opreation of two VMs like:
> > > >
> > > > 1) Malicious VM1 allocate 1 byte rx buffer and try to slow down the
> > > > vhost process as much as possible e.g using indirect descriptors or
> > > > other.
> > > > 2) Malicious VM2 generate packets to VM1 as fast as possible
> > > >
> > > > Fixing this by checking against weight at the end of RX and TX
> > > > loop. This also eliminate other similar cases when:
> > > >
> > > > - userspace is consuming the packets in the meanwhile
> > > > - theoretical TOCTOU attack if guest moving avail index back and forth
> > > > to hit the continue after vhost find guest just add new buffers
> > > >
> > > > This addresses CVE-2019-3900.
> > > >
> > > > Fixes: d8316f3991d20 ("vhost: fix total length when packets are
> > > > too short")
> > > I agree this is the real issue.
> > >
> > > > Fixes: 3a4d5c94e9593 ("vhost_net: a kernel-level virtio server")
> > > This is just a red herring imho. We can stick this on any vhost patch :)
> > >
> > > > Signed-off-by: Jason Wang
> > > > ---
> > > > drivers/vhost/net.c | 41 +
> > > > 1 file changed, 21 insertions(+), 20 deletions(-)
> > > >
> > > > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> > > > index df51a35..fb46e6b 100644
> > > > --- a/drivers/vhost/net.c
> > > > +++ b/drivers/vhost/net.c
> > > > @@ -778,8 +778,9 @@ static void handle_tx_copy(struct vhost_net
> > > > *net, struct socket *sock)
> > > > int err;
> > > > int sent_pkts = 0;
> > > > bool sock_can_batch = (sock->sk->sk_sndbuf == INT_MAX);
> > > > + bool next_round = false;
> > > > - for (;;) {
> > > > + do {
> > > > bool busyloop_intr = false;
> > > > if (nvq->done_idx == VHOST_NET_BATCH)
> > > > @@ -845,11 +846,10 @@ static void handle_tx_copy(struct
> > > > vhost_net *net, struct socket *sock)
> > > > vq->heads[nvq->done_idx].id = cpu_to_vhost32(vq, head);
> > > > vq->heads[nvq->done_idx].len = 0;
> > > > ++nvq->done_idx;
> > > > - if (vhost_exceeds_weight(++sent_pkts, total_len)) {
> > > > - vhost_poll_queue(>poll);
> > > > - break;
> > > > - }
> > > > - }
> > > > + } while (!(next_round = vhost_exceeds_weight(++sent_pkts,
> > > > total_len)));
> > > > +
> > > > + if (next_round)
> > > > + vhost_poll_queue(>poll);
> > > > vhost_tx_batch(net, nvq, sock, );
> > > > }
> > > > @@ -873,8 +873,9 @@ static void handle_tx_zerocopy(struct
> > > > vhost_net *net, struct socket *sock)
> > > > struct vhost_net_ubuf_ref *uninitialized_var(ubufs);
> > > > bool zcopy_used;
> > > > int sent_pkts = 0;
> > > > + bool next_round = false;
> > > > - for (;;) {
> > > > + do {
> > > > bool busyloop_intr;
> > > > /* Release DMAs done buffers first */
> > > > @@ -951,11 +952,10 @@ static void handle_tx_zerocopy(struct
> > > > vhost_net *net, struct socket *sock)
> > > > else
> > > > vhost_zerocopy_signal_used(net, vq);
> > > > vhost_net_tx_packet(net);
> > > > - if (unlikely(vhost_exceeds_weight(++sent_pkts, total_len))) {
> > > > - vhost_poll_queue(>poll);
> > > > - break;
> > > > - }
> > > > - }
> > > > + } while (!(next_round = vhost_exceeds_weight(++sent_pkts,
> > > > total_len)));
> > > > +
> > > > + if (next_round)
> > > > + vhost_poll_queue(>poll);
> > > > }
> > > > /* Expects to be always run from workqueue - which acts as
> > > > @@ -1134,6 +1134,7 @@ static void handle_rx(struct vhost_net *net)
> > > > struct iov_iter fixup;
> > > > __virtio16 num_buffers;
> > > > int recv_pkts = 0;
> > > > + bool next_round = false;
> > > > mutex_lock_nested(>mutex, VHOST_NET_VQ_RX);
> > > > sock = vq->private_data;
> > > > @@ -1153,8 +1154,11 @@ static void handle_rx(struct vhost_net *net)
> > > > vq->log : NULL;
> > >
Re: [PATCH v2 1/8] vsock/virtio: limit the memory used per-socket
On Fri, May 10, 2019 at 02:58:36PM +0200, Stefano Garzarella wrote:
> Since virtio-vsock was introduced, the buffers filled by the host
> and pushed to the guest using the vring, are directly queued in
> a per-socket list avoiding to copy it.
> These buffers are preallocated by the guest with a fixed
> size (4 KB).
>
> The maximum amount of memory used by each socket should be
> controlled by the credit mechanism.
> The default credit available per-socket is 256 KB, but if we use
> only 1 byte per packet, the guest can queue up to 262144 of 4 KB
> buffers, using up to 1 GB of memory per-socket. In addition, the
> guest will continue to fill the vring with new 4 KB free buffers
> to avoid starvation of other sockets.
>
> This patch solves this issue copying the payload in a new buffer.
> Then it is queued in the per-socket list, and the 4KB buffer used
> by the host is freed.
>
> In this way, the memory used by each socket respects the credit
> available, and we still avoid starvation, paying the cost of an
> extra memory copy. When the buffer is completely full we do a
> "zero-copy", moving the buffer directly in the per-socket list.
>
> Signed-off-by: Stefano Garzarella
> ---
> drivers/vhost/vsock.c | 2 +
> include/linux/virtio_vsock.h| 8 +++
> net/vmw_vsock/virtio_transport.c| 1 +
> net/vmw_vsock/virtio_transport_common.c | 95 ++---
> 4 files changed, 81 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> index bb5fc0e9fbc2..7964e2daee09 100644
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c
> @@ -320,6 +320,8 @@ vhost_vsock_alloc_pkt(struct vhost_virtqueue *vq,
> return NULL;
> }
>
> + pkt->buf_len = pkt->len;
> +
> nbytes = copy_from_iter(pkt->buf, pkt->len, _iter);
> if (nbytes != pkt->len) {
> vq_err(vq, "Expected %u byte payload, got %zu bytes\n",
> diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
> index e223e2632edd..345f04ee9193 100644
> --- a/include/linux/virtio_vsock.h
> +++ b/include/linux/virtio_vsock.h
> @@ -54,9 +54,17 @@ struct virtio_vsock_pkt {
> void *buf;
> u32 len;
> u32 off;
> + u32 buf_len;
> bool reply;
> };
>
> +struct virtio_vsock_buf {
> + struct list_head list;
> + void *addr;
> + u32 len;
> + u32 off;
> +};
> +
> struct virtio_vsock_pkt_info {
> u32 remote_cid, remote_port;
> struct vsock_sock *vsk;
> diff --git a/net/vmw_vsock/virtio_transport.c
> b/net/vmw_vsock/virtio_transport.c
> index 15eb5d3d4750..af1d2ce12f54 100644
> --- a/net/vmw_vsock/virtio_transport.c
> +++ b/net/vmw_vsock/virtio_transport.c
> @@ -280,6 +280,7 @@ static void virtio_vsock_rx_fill(struct virtio_vsock
> *vsock)
> break;
> }
>
> + pkt->buf_len = buf_len;
> pkt->len = buf_len;
>
> sg_init_one(, >hdr, sizeof(pkt->hdr));
> diff --git a/net/vmw_vsock/virtio_transport_common.c
> b/net/vmw_vsock/virtio_transport_common.c
> index 602715fc9a75..0248d6808755 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -65,6 +65,9 @@ virtio_transport_alloc_pkt(struct virtio_vsock_pkt_info
> *info,
> pkt->buf = kmalloc(len, GFP_KERNEL);
> if (!pkt->buf)
> goto out_pkt;
> +
> + pkt->buf_len = len;
> +
> err = memcpy_from_msg(pkt->buf, info->msg, len);
> if (err)
> goto out;
> @@ -86,6 +89,46 @@ virtio_transport_alloc_pkt(struct virtio_vsock_pkt_info
> *info,
> return NULL;
> }
>
> +static struct virtio_vsock_buf *
> +virtio_transport_alloc_buf(struct virtio_vsock_pkt *pkt, bool zero_copy)
> +{
> + struct virtio_vsock_buf *buf;
> +
> + if (pkt->len == 0)
> + return NULL;
> +
> + buf = kzalloc(sizeof(*buf), GFP_KERNEL);
> + if (!buf)
> + return NULL;
> +
> + /* If the buffer in the virtio_vsock_pkt is full, we can move it to
> + * the new virtio_vsock_buf avoiding the copy, because we are sure that
> + * we are not use
we do not use
> more memory than that counted by the credit mechanism.
> + */
> + if (zero_copy && pkt->len == pkt->buf_len) {
> + buf->addr = pkt->buf;
> + pkt->buf = NULL;
> + } else {
> + buf->addr = kmalloc(pkt->len, GFP_KERNEL);
> + if (!buf->addr) {
> + kfree(buf);
> + return NULL;
> + }
> +
> + memcpy(buf->addr, pkt->buf, pkt->len);
> + }
> +
> + buf->len = pkt->len;
> +
> + return buf;
> +}
> +
> +static void virtio_transport_free_buf(struct virtio_vsock_buf *buf)
> +{
> + kfree(buf->addr);
> + kfree(buf);
> +}
> +
> /* Packet capture */
> static struct sk_buff
Re: [PATCH v8 0/6] virtio pmem driver
On Fri, May 10, 2019 at 07:33:03PM -0400, Pankaj Gupta wrote: > > > > > > > Hi Michael & Dan, > > > > > > Please review/ack the patch series from LIBNVDIMM & VIRTIO side. > > > We have ack on ext4, xfs patches(4, 5 & 6) patch 2. Still need > > > your ack on nvdimm patches(1 & 3) & virtio patch 2. > > > > I was planning to merge these via the nvdimm tree, not ack them. Did > > you have another maintainer lined up to take these patches? > > Sorry! for not being clear on this. I wanted to say same. > > Proposed the patch series to be merged via nvdimm tree as kindly agreed > by you. We only need an ack on virtio patch 2 from Micahel. > > Thank you for all your help. > > Best regards, > Pankaj Gupta Fine by me. > > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v8 2/6] virtio-pmem: Add virtio pmem driver
On Fri, May 10, 2019 at 09:21:58PM +0530, Pankaj Gupta wrote:
> This patch adds virtio-pmem driver for KVM guest.
>
> Guest reads the persistent memory range information from
> Qemu over VIRTIO and registers it on nvdimm_bus. It also
> creates a nd_region object with the persistent memory
> range information so that existing 'nvdimm/pmem' driver
> can reserve this into system memory map. This way
> 'virtio-pmem' driver uses existing functionality of pmem
> driver to register persistent memory compatible for DAX
> capable filesystems.
>
> This also provides function to perform guest flush over
> VIRTIO from 'pmem' driver when userspace performs flush
> on DAX memory range.
>
> Signed-off-by: Pankaj Gupta
> Reviewed-by: Yuval Shaia
Acked-by: Michael S. Tsirkin
> ---
> drivers/nvdimm/Makefile | 1 +
> drivers/nvdimm/nd_virtio.c | 129 +++
> drivers/nvdimm/virtio_pmem.c | 117
> drivers/virtio/Kconfig | 10 +++
> include/linux/virtio_pmem.h | 60 ++
> include/uapi/linux/virtio_ids.h | 1 +
> include/uapi/linux/virtio_pmem.h | 10 +++
> 7 files changed, 328 insertions(+)
> create mode 100644 drivers/nvdimm/nd_virtio.c
> create mode 100644 drivers/nvdimm/virtio_pmem.c
> create mode 100644 include/linux/virtio_pmem.h
> create mode 100644 include/uapi/linux/virtio_pmem.h
>
> diff --git a/drivers/nvdimm/Makefile b/drivers/nvdimm/Makefile
> index 6f2a088afad6..cefe233e0b52 100644
> --- a/drivers/nvdimm/Makefile
> +++ b/drivers/nvdimm/Makefile
> @@ -5,6 +5,7 @@ obj-$(CONFIG_ND_BTT) += nd_btt.o
> obj-$(CONFIG_ND_BLK) += nd_blk.o
> obj-$(CONFIG_X86_PMEM_LEGACY) += nd_e820.o
> obj-$(CONFIG_OF_PMEM) += of_pmem.o
> +obj-$(CONFIG_VIRTIO_PMEM) += virtio_pmem.o nd_virtio.o
>
> nd_pmem-y := pmem.o
>
> diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
> new file mode 100644
> index ..ed7ddcc5a62c
> --- /dev/null
> +++ b/drivers/nvdimm/nd_virtio.c
> @@ -0,0 +1,129 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * virtio_pmem.c: Virtio pmem Driver
> + *
> + * Discovers persistent memory range information
> + * from host and provides a virtio based flushing
> + * interface.
> + */
> +#include
> +#include "nd.h"
> +
> + /* The interrupt handler */
> +void host_ack(struct virtqueue *vq)
> +{
> + unsigned int len;
> + unsigned long flags;
> + struct virtio_pmem_request *req, *req_buf;
> + struct virtio_pmem *vpmem = vq->vdev->priv;
> +
> + spin_lock_irqsave(>pmem_lock, flags);
> + while ((req = virtqueue_get_buf(vq, )) != NULL) {
> + req->done = true;
> + wake_up(>host_acked);
> +
> + if (!list_empty(>req_list)) {
> + req_buf = list_first_entry(>req_list,
> + struct virtio_pmem_request, list);
> + req_buf->wq_buf_avail = true;
> + wake_up(_buf->wq_buf);
> + list_del(_buf->list);
> + }
> + }
> + spin_unlock_irqrestore(>pmem_lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(host_ack);
> +
> + /* The request submission function */
> +int virtio_pmem_flush(struct nd_region *nd_region)
> +{
> + int err, err1;
> + unsigned long flags;
> + struct scatterlist *sgs[2], sg, ret;
> + struct virtio_device *vdev = nd_region->provider_data;
> + struct virtio_pmem *vpmem = vdev->priv;
> + struct virtio_pmem_request *req;
> +
> + might_sleep();
> + req = kmalloc(sizeof(*req), GFP_KERNEL);
> + if (!req)
> + return -ENOMEM;
> +
> + req->done = false;
> + strcpy(req->name, "FLUSH");
> + init_waitqueue_head(>host_acked);
> + init_waitqueue_head(>wq_buf);
> + INIT_LIST_HEAD(>list);
> + sg_init_one(, req->name, strlen(req->name));
> + sgs[0] =
> + sg_init_one(, >ret, sizeof(req->ret));
> + sgs[1] =
> +
> + spin_lock_irqsave(>pmem_lock, flags);
> + /*
> + * If virtqueue_add_sgs returns -ENOSPC then req_vq virtual
> + * queue does not have free descriptor. We add the request
> + * to req_list and wait for host_ack to wake us up when free
> + * slots are available.
> + */
> + while ((err = virtqueue_add_sgs(vpmem->req_vq, sgs, 1, 1, req,
> + GFP_ATOMIC)) == -ENOSPC) {
> +
> + dev_err(>dev, "failed to send command to virtio pmem"\
> + "device, no free slots in the virtqueue\n");
> + req->wq_buf_avail = false;
> + list_add_tail(>list, >req_list);
> + spin_unlock_irqrestore(>pmem_lock, flags);
> +
> + /* When host has read buffer, this completes via host_ack */
> + wait_event(req->wq_buf, req->wq_buf_avail);
> + spin_lock_irqsave(>pmem_lock, flags);
> + }
> + err1 = virtqueue_kick(vpmem->req_vq);
> + spin_unlock_irqrestore(>pmem_lock,
Re: [PATCH 01/10] virtio/s390: use vring_create_virtqueue
On Fri, May 10, 2019 at 04:07:44PM +0200, Cornelia Huck wrote:
> On Tue, 7 May 2019 15:58:12 +0200
> Christian Borntraeger wrote:
>
> > On 05.05.19 13:15, Cornelia Huck wrote:
> > > On Sat, 4 May 2019 16:03:40 +0200
> > > Halil Pasic wrote:
> > >
> > >> On Fri, 3 May 2019 16:04:48 -0400
> > >> "Michael S. Tsirkin" wrote:
> > >>
> > >>> On Fri, May 03, 2019 at 11:17:24AM +0200, Cornelia Huck wrote:
> > On Fri, 26 Apr 2019 20:32:36 +0200
> > Halil Pasic wrote:
> >
> > > The commit 2a2d1382fe9d ("virtio: Add improved queue allocation API")
> > > establishes a new way of allocating virtqueues (as a part of the
> > > effort
> > > that taught DMA to virtio rings).
> > >
> > > In the future we will want virtio-ccw to use the DMA API as well.
> > >
> > > Let us switch from the legacy method of allocating virtqueues to
> > > vring_create_virtqueue() as the first step into that direction.
> > >
> > > Signed-off-by: Halil Pasic
> > > ---
> > > drivers/s390/virtio/virtio_ccw.c | 30 +++---
> > > 1 file changed, 11 insertions(+), 19 deletions(-)
> >
> > Reviewed-by: Cornelia Huck
> >
> > I'd vote for merging this patch right away for 5.2.
> > >>>
> > >>> So which tree is this going through? mine?
> > >>>
> > >>
> > >> Christian, what do you think? If the whole series is supposed to go in
> > >> in one go (which I hope it is), via Martin's tree could be the simplest
> > >> route IMHO.
> > >
> > >
> > > The first three patches are virtio(-ccw) only and the those are the ones
> > > that I think are ready to go.
> > >
> > > I'm not feeling comfortable going forward with the remainder as it
> > > stands now; waiting for some other folks to give feedback. (They are
> > > touching/interacting with code parts I'm not so familiar with, and lack
> > > of documentation, while not the developers' fault, does not make it
> > > easier.)
> > >
> > > Michael, would you like to pick up 1-3 for your tree directly? That
> > > looks like the easiest way.
> >
> > Agreed. Michael please pick 1-3.
> > We will continue to review 4- first and then see which tree is best.
>
> Michael, please let me know if you'll pick directly or whether I should
> post a series.
>
> [Given that the patches are from one virtio-ccw maintainer and reviewed
> by the other, picking directly would eliminate an unnecessary
> indirection :)]
picked them
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 0/7] Add virtio-iommu driver
On Tue, Jan 15, 2019 at 12:19:52PM +, Jean-Philippe Brucker wrote: > Implement the virtio-iommu driver, following specification v0.9 [1]. > > This is a simple rebase onto Linux v5.0-rc2. We now use the > dev_iommu_fwspec_get() helper introduced in v5.0 instead of accessing > dev->iommu_fwspec, but there aren't any functional change from v6 [2]. > > Our current goal for virtio-iommu is to get a paravirtual IOMMU working > on Arm, and enable device assignment to guest userspace. In this > use-case the mappings are static, and don't require optimal performance, > so this series tries to keep things simple. However there is plenty more > to do for features and optimizations, and having this base in v5.1 would > be good. Given that most of the changes are to drivers/iommu, I believe > the driver and future changes should go via the IOMMU tree. > > You can find Linux driver and kvmtool device on v0.9.2 branches [3], > module and x86 support on virtio-iommu/devel. Also tested with Eric's > QEMU device [4]. Please note that the series depends on Robin's > probe-deferral fix [5], which will hopefully land in v5.0. > > [1] Virtio-iommu specification v0.9, sources and pdf > git://linux-arm.org/virtio-iommu.git virtio-iommu/v0.9 > http://jpbrucker.net/virtio-iommu/spec/v0.9/virtio-iommu-v0.9.pdf > > [2] [PATCH v6 0/7] Add virtio-iommu driver > > https://lists.linuxfoundation.org/pipermail/iommu/2018-December/032127.html > > [3] git://linux-arm.org/linux-jpb.git virtio-iommu/v0.9.2 > git://linux-arm.org/kvmtool-jpb.git virtio-iommu/v0.9.2 > > [4] [RFC v9 00/17] VIRTIO-IOMMU device > https://www.mail-archive.com/qemu-devel@nongnu.org/msg575578.html > > [5] [PATCH] iommu/of: Fix probe-deferral > https://www.spinics.net/lists/arm-kernel/msg698371.html OK this has been in next for a while. Last time IOMMU maintainers objected. Are objections still in force? If not could we get acks please? > Jean-Philippe Brucker (7): > dt-bindings: virtio-mmio: Add IOMMU description > dt-bindings: virtio: Add virtio-pci-iommu node > of: Allow the iommu-map property to omit untranslated devices > PCI: OF: Initialize dev->fwnode appropriately > iommu: Add virtio-iommu driver > iommu/virtio: Add probe request > iommu/virtio: Add event queue > > .../devicetree/bindings/virtio/iommu.txt | 66 + > .../devicetree/bindings/virtio/mmio.txt | 30 + > MAINTAINERS |7 + > drivers/iommu/Kconfig | 11 + > drivers/iommu/Makefile|1 + > drivers/iommu/virtio-iommu.c | 1158 + > drivers/of/base.c | 10 +- > drivers/pci/of.c |7 + > include/uapi/linux/virtio_ids.h |1 + > include/uapi/linux/virtio_iommu.h | 161 +++ > 10 files changed, 1449 insertions(+), 3 deletions(-) > create mode 100644 Documentation/devicetree/bindings/virtio/iommu.txt > create mode 100644 drivers/iommu/virtio-iommu.c > create mode 100644 include/uapi/linux/virtio_iommu.h > > -- > 2.19.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
