similar to: [PATCH] vhost: Don't call vq_access_ok() when using IOTLB

On Mon, Sep 28, 2020 at 02:35:04PM +0200, Greg Kurz wrote: > When the IOTLB device is enabled, the vring addresses we get from > userspace are GIOVAs. It is thus wrong to pass them to vq_access_ok() > which only takes HVAs. The IOTLB map is likely empty at this stage, > so there isn't much that can be done with these GIOVAs. Access validation > will be performed at IOTLB

On Tue, Sep 29, 2020 at 06:30:20PM +0200, Greg Kurz wrote: > This series addresses some misuse around vring addresses provided by > userspace when using an IOTLB device. The misuse cause failures of > the VHOST_SET_VRING_ADDR ioctl on POWER, which in turn causes QEMU > to crash at migration time. > > While digging some more I realized that log_access_ok() can also be >

Vq log_base is the userspace address of bitmap which has nothing to do with IOTLB. So it needs to be validated unconditionally otherwise we may try use 0 as log_base which may lead to pin pages that will lead unexpected result (e.g trigger BUG_ON() in set_bit_to_user()). Fixes: 6b1e6cc7855b0 ("vhost: new device IOTLB API") Reported-by: syzbot+6304bf97ef436580fede at

Vq log_base is the userspace address of bitmap which has nothing to do with IOTLB. So it needs to be validated unconditionally otherwise we may try use 0 as log_base which may lead to pin pages that will lead unexpected result (e.g trigger BUG_ON() in set_bit_to_user()). Fixes: 6b1e6cc7855b0 ("vhost: new device IOTLB API") Reported-by: syzbot+6304bf97ef436580fede at

On Thu, Mar 29, 2018 at 04:00:04PM +0800, Jason Wang wrote: > Vq log_base is the userspace address of bitmap which has nothing to do > with IOTLB. So it needs to be validated unconditionally otherwise we > may try use 0 as log_base which may lead to pin pages that will lead > unexpected result (e.g trigger BUG_ON() in set_bit_to_user()). > > Fixes: 6b1e6cc7855b0 ("vhost:

On Thu, Mar 29, 2018 at 04:00:04PM +0800, Jason Wang wrote: > Vq log_base is the userspace address of bitmap which has nothing to do > with IOTLB. So it needs to be validated unconditionally otherwise we > may try use 0 as log_base which may lead to pin pages that will lead > unexpected result (e.g trigger BUG_ON() in set_bit_to_user()). > > Fixes: 6b1e6cc7855b0 ("vhost:

This patch tries to implement an device IOTLB for vhost. This could be used with for co-operation with userspace(qemu) implementation of DMA remapping. The idea is simple, cache the translation in a software device IOTLB (which was implemented as interval tree) in vhost and use vhost_net file descriptor for reporting IOTLB miss and IOTLB update/invalidation. When vhost meets an IOTLB miss, the

On 01/30/2017 08:06 PM, Greg Kurz wrote: >> Currently, under certain circumstances vhost_init_is_le does just a part >> of the initialization job, and depends on vhost_reset_is_le being called >> too. For this reason vhost_vq_init_access used to call vhost_reset_is_le >> when vq->private_data is NULL. This is not only counter intuitive, but >> also real a problem

On 01/30/2017 08:06 PM, Greg Kurz wrote: >> Currently, under certain circumstances vhost_init_is_le does just a part >> of the initialization job, and depends on vhost_reset_is_le being called >> too. For this reason vhost_vq_init_access used to call vhost_reset_is_le >> when vq->private_data is NULL. This is not only counter intuitive, but >> also real a problem

On Mon, 30 Jan 2017 11:09:36 +0100 Halil Pasic <pasic at linux.vnet.ibm.com> wrote: > Currently, under certain circumstances vhost_init_is_le does just a part > of the initialization job, and depends on vhost_reset_is_le being called > too. For this reason vhost_vq_init_access used to call vhost_reset_is_le > when vq->private_data is NULL. This is not only counter intuitive,

On Tue, Jan 31, 2017 at 04:56:13PM +0100, Halil Pasic wrote: > > > On 01/30/2017 08:06 PM, Greg Kurz wrote: > >> Currently, under certain circumstances vhost_init_is_le does just a part > >> of the initialization job, and depends on vhost_reset_is_le being called > >> too. For this reason vhost_vq_init_access used to call vhost_reset_is_le > >> when

With PUBLISH_USED_IDX, guest tells us which used entries it has consumed. This can be used to reduce the number of interrupts: after we write a used entry, if the guest has not yet consumed the previous entry, or if the guest has already consumed the new entry, we do not need to interrupt. This imporves bandwidth by 30% under some workflows. Signed-off-by: Michael S. Tsirkin <mst at

With PUBLISH_USED_IDX, guest tells us which used entries it has consumed. This can be used to reduce the number of interrupts: after we write a used entry, if the guest has not yet consumed the previous entry, or if the guest has already consumed the new entry, we do not need to interrupt. This imporves bandwidth by 30% under some workflows. Signed-off-by: Michael S. Tsirkin <mst at

With PUBLISH_USED_IDX, guest tells us which used entries it has consumed. This can be used to reduce the number of interrupts: after we write a used entry, if the guest has not yet consumed the previous entry, or if the guest has already consumed the new entry, we do not need to interrupt. This imporves bandwidth by 30% under some workflows. Signed-off-by: Michael S. Tsirkin <mst at

With PUBLISH_USED_IDX, guest tells us which used entries it has consumed. This can be used to reduce the number of interrupts: after we write a used entry, if the guest has not yet consumed the previous entry, or if the guest has already consumed the new entry, we do not need to interrupt. This imporves bandwidth by 30% under some workflows. Signed-off-by: Michael S. Tsirkin <mst at

Refactor code to make sure features are only accessed under VQ mutex. This makes everything simpler, no need for RCU here anymore. Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- Note: this is on top of my last pull request drivers/vhost/vhost.h | 11 +++-------- drivers/vhost/net.c | 8 +++----- drivers/vhost/scsi.c | 22 +++++++++++++--------- drivers/vhost/test.c | 9

Refactor code to make sure features are only accessed under VQ mutex. This makes everything simpler, no need for RCU here anymore. Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- Note: this is on top of my last pull request drivers/vhost/vhost.h | 11 +++-------- drivers/vhost/net.c | 8 +++----- drivers/vhost/scsi.c | 22 +++++++++++++--------- drivers/vhost/test.c | 9

On biarch kernels, it is not safe to do copy from/to user, even with use_mm, unless we checked the address range with access_ok previously. Implement these checks to enforce safe memory accesses. Reported-by: Al Viro <viro at zeniv.linux.org.uk> Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- drivers/vhost/net.c | 17 ++++++- drivers/vhost/vhost.c | 111

On biarch kernels, it is not safe to do copy from/to user, even with use_mm, unless we checked the address range with access_ok previously. Implement these checks to enforce safe memory accesses. Reported-by: Al Viro <viro at zeniv.linux.org.uk> Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- drivers/vhost/net.c | 17 ++++++- drivers/vhost/vhost.c | 111

Currently vhost *_access_ok() functions return int. This is error-prone because there are two popular conventions: 1. 0 means failure, 1 means success 2. -errno means failure, 0 means success Although vhost mostly uses #1, it does not do so consistently. umem_access_ok() uses #2. This patch changes the return type from int to bool so that false means failure and true means success. This