search for: translate_desc

When fail, translate_desc() returns negative value, otherwise the number of iovs. So we should fail when the return value is negative instead of a blindly check against zero. Reported-by: Stephen Hemminger <stephen at networkplumber.org> Fixes: cc5e71075947 ("vhost: log dirty page correctly") Signed-off-by:...

When fail, translate_desc() returns negative value, otherwise the number of iovs. So we should fail when the return value is negative instead of a blindly check against zero. Reported-by: Stephen Hemminger <stephen at networkplumber.org> Fixes: cc5e71075947 ("vhost: log dirty page correctly") Signed-off-by:...

When fail, translate_desc() returns negative value, otherwise the number of iovs. So we should fail when the return value is negative instead of a blindly check against zero. Detected by CoverityScan, CID# 1442593: Control flow issues (DEADCODE) Fixes: cc5e71075947 ("vhost: log dirty page correctly") Acked-by:...

...ote: > Il 02/06/2014 23:58, Eric Dumazet ha scritto: > > This looks dubious > > > > What about using kfree_rcu() instead ? > > It would lead to unbound allocation from userspace. Look at how we did this in commit c3059477fce2d956a0bb3e04357324780c5d8eeb > > > translate_desc() still uses rcu_read_lock(), its not clear if the mutex > > is really held. > > Yes, vhost_get_vq_desc must be called with the vq mutex held. > > The rcu_read_lock/unlock in translate_desc is unnecessary. Yep, this is what I pointed out. This is not only necessary, but confusi...

...ote: > Il 02/06/2014 23:58, Eric Dumazet ha scritto: > > This looks dubious > > > > What about using kfree_rcu() instead ? > > It would lead to unbound allocation from userspace. Look at how we did this in commit c3059477fce2d956a0bb3e04357324780c5d8eeb > > > translate_desc() still uses rcu_read_lock(), its not clear if the mutex > > is really held. > > Yes, vhost_get_vq_desc must be called with the vq mutex held. > > The rcu_read_lock/unlock in translate_desc is unnecessary. Yep, this is what I pointed out. This is not only necessary, but confusi...

...alent of synchronize_rcu() > + */ > + for (i = 0; i < d->nvqs; ++i) { > + mutex_lock(&d->vqs[i]->mutex); > + mutex_unlock(&d->vqs[i]->mutex); > + } > kfree(oldmem); > return 0; > } This looks dubious What about using kfree_rcu() instead ? translate_desc() still uses rcu_read_lock(), its not clear if the mutex is really held.

...alent of synchronize_rcu() > + */ > + for (i = 0; i < d->nvqs; ++i) { > + mutex_lock(&d->vqs[i]->mutex); > + mutex_unlock(&d->vqs[i]->mutex); > + } > kfree(oldmem); > return 0; > } This looks dubious What about using kfree_rcu() instead ? translate_desc() still uses rcu_read_lock(), its not clear if the mutex is really held.

From: Jason Wang <jasowang at redhat.com> Date: Fri, 15 Feb 2019 15:53:24 +0800 > When fail, translate_desc() returns negative value, otherwise the > number of iovs. So we should fail when the return value is negative > instead of a blindly check against zero. > > Reported-by: Stephen Hemminger <stephen at networkplumber.org> > Fixes: cc5e71075947 ("vhost: log dirty page correc...

On Fri, 15 Feb 2019 15:53:24 +0800 Jason Wang <jasowang at redhat.com> wrote: > When fail, translate_desc() returns negative value, otherwise the > number of iovs. So we should fail when the return value is negative > instead of a blindly check against zero. > > Reported-by: Stephen Hemminger <stephen at networkplumber.org> > Fixes: cc5e71075947 ("vhost: log dirty page correc...

From: Colin Ian King <colin.king at canonical.com> Currently the loop that calls log_write_hva on each iovec is never executed because of an incorrect error check on the return from the call to translate_desc. The check should be checking for a -ve error return and because it makes no sense to iterate over zero items, the checks should also check for zero too. Detected by CoverityScan, CID#1476969 ("Logically dead code") Fixes: cc5e71075947 ("vhost: log dirty page correctly") Sign...

From: Colin Ian King <colin.king at canonical.com> Currently the loop that calls log_write_hva on each iovec is never executed because of an incorrect error check on the return from the call to translate_desc. The check should be checking for a -ve error return and because it makes no sense to iterate over zero items, the checks should also check for zero too. Detected by CoverityScan, CID#1476969 ("Logically dead code") Fixes: cc5e71075947 ("vhost: log dirty page correctly") Sign...

...;vq->used->idx, sizeof vq->used->idx)) + return -EFAULT; + r = __get_user(last_used_idx, &vq->used->idx); + if (r) + return r; + vq->last_used_idx = vhost16_to_cpu(vq, last_used_idx); + return 0; } EXPORT_SYMBOL_GPL(vhost_init_used); @@ -1087,16 +1094,16 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len, /* Each buffer in the virtqueues is actually a chain of descriptors. This * function returns the next descriptor in the chain, * or -1U if we're at the end. */ -static unsigned next_desc(struct vring_desc *desc) +static unsigned next_desc(str...

...;vq->used->idx, sizeof vq->used->idx)) + return -EFAULT; + r = __get_user(last_used_idx, &vq->used->idx); + if (r) + return r; + vq->last_used_idx = vhost16_to_cpu(vq, last_used_idx); + return 0; } EXPORT_SYMBOL_GPL(vhost_init_used); @@ -1087,16 +1094,16 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len, /* Each buffer in the virtqueues is actually a chain of descriptors. This * function returns the next descriptor in the chain, * or -1U if we're at the end. */ -static unsigned next_desc(struct vring_desc *desc) +static unsigned next_desc(str...

From: Hongyong Zang <zanghongyong at huawei.com> Vhost-net uses its own vhost_memory, which results from user space (qemu) info, to translate GPA to HVA. Since kernel's kvm structure already maintains the address relationship in its member *kvm_memslots*, these patches use kernel's kvm_memslots directly without the need of initialization and maintenance of vhost_memory. Hongyong

From: Hongyong Zang <zanghongyong at huawei.com> Vhost-net uses its own vhost_memory, which results from user space (qemu) info, to translate GPA to HVA. Since kernel's kvm structure already maintains the address relationship in its member *kvm_memslots*, these patches use kernel's kvm_memslots directly without the need of initialization and maintenance of vhost_memory. Hongyong

...;vq->used->idx, sizeof vq->used->idx)) + return -EFAULT; + r = __get_user(last_used_idx, &vq->used->idx); + if (r) + return r; + vq->last_used_idx = vhost16_to_cpu(vq, last_used_idx); + return 0; } EXPORT_SYMBOL_GPL(vhost_init_used); @@ -1087,16 +1094,16 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len, /* Each buffer in the virtqueues is actually a chain of descriptors. This * function returns the next descriptor in the chain, * or -1U if we're at the end. */ -static unsigned next_desc(struct vring_desc *desc) +static unsigned next_desc(str...

...;vq->used->idx, sizeof vq->used->idx)) + return -EFAULT; + r = __get_user(last_used_idx, &vq->used->idx); + if (r) + return r; + vq->last_used_idx = vhost16_to_cpu(vq, last_used_idx); + return 0; } EXPORT_SYMBOL_GPL(vhost_init_used); @@ -1087,16 +1094,16 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len, /* Each buffer in the virtqueues is actually a chain of descriptors. This * function returns the next descriptor in the chain, * or -1U if we're at the end. */ -static unsigned next_desc(struct vring_desc *desc) +static unsigned next_desc(str...

Building vhost on 32-bit targets results in the following error. drivers/vhost/vhost.c: In function 'translate_desc': include/linux/compiler.h:549:38: error: call to '__compiletime_assert_1879' declared with attribute error: BUILD_BUG_ON failed: sizeof(_s) > sizeof(long) Fixes: a89db445fbd7 ("vhost: block speculation of translated descriptors") Cc: Michael S. Tsirkin <mst at redhat...

...sed(struct vhost_virtqueue *vq) r = __get_user(last_used_idx, &vq->used->idx); if (r) return r; - vq->last_used_idx = last_used_idx; + vq->last_used_idx = vhost16_to_cpu(vq, last_used_idx); return 0; } EXPORT_SYMBOL_GPL(vhost_init_used); @@ -1094,16 +1094,16 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len, /* Each buffer in the virtqueues is actually a chain of descriptors. This * function returns the next descriptor in the chain, * or -1U if we're at the end. */ -static unsigned next_desc(struct vring_desc *desc) +static unsigned next_desc(str...

...sed(struct vhost_virtqueue *vq) r = __get_user(last_used_idx, &vq->used->idx); if (r) return r; - vq->last_used_idx = last_used_idx; + vq->last_used_idx = vhost16_to_cpu(vq, last_used_idx); return 0; } EXPORT_SYMBOL_GPL(vhost_init_used); @@ -1094,16 +1094,16 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len, /* Each buffer in the virtqueues is actually a chain of descriptors. This * function returns the next descriptor in the chain, * or -1U if we're at the end. */ -static unsigned next_desc(struct vring_desc *desc) +static unsigned next_desc(str...