Thread (4 messages) flat view 4 messages, 2 authors, 2021-02-22

Re: Kernel panic with vhost-vdpa

From: Jason Wang <hidden>
Date: 2021-02-18 08:48:46

Hi Gautam:

On 2021/2/15 9:01 下午, Gautam Dawar wrote:
Hi Jason/Michael,

I observed a kernel panic while testing vhost-vdpa with Xilinx 
adapters. Here are the details for your review:

Problem statement:

When qemu with vhost-vdpa netdevice is run for the first time, it 
works well. But after the VM is powered off, next qemu run causes 
kernel panic due to a NULL pointer dereference in 
irq_bypass_register_producer().

Root cause analysis:

When the VM is powered off, vhost_dev_stop() is invoked which in turn 
calls vhost_vdpa_reset_device() causing the irq_bypass producers to be 
unregistered.

On the next run, when qemu opens the vhost device, the 
vhost_vdpa_open() file operation calls vhost_dev_init(). Here, 
call_ctx->producer memory is cleared in vhost_vring_call_reset().

Further, when the virtqueues are initialized by 
vhost_virtqueue_init(), vhost_vdpa_setup_vq_irq() again registers the 
irq_bypass producer for each virtqueue. As the node member of struct 
irq_bypass_producer is also initialized to zero, traversal on the 
producers list causes crash due to NULL pointer dereference.
Thanks a lot for reporting this issue.

quoted hunk
Fix details:

I think that this issue can be fixed by invoking 
vhost_vdpa_setup_vq_irq() only when vhost_vdpa_set_status() includes 
VIRTIO_CONFIG_S_DRIVER_OK in the new status value. This way, there 
won’t be any stale nodes in the irqbypass  module’s producers list 
which are reset in vhost_vring_call_reset().

Patch:
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c index 
62a9bb0efc55..fdad94e2fbf9 100644
--- a/drivers/vhost/vdpa.c

+++ b/drivers/vhost/vdpa.c
@@ -409,7 +409,6 @@ static long vhost_vdpa_vring_ioctl(struct 
vhost_vdpa *v, unsigned int cmd,

cb.private = NULL;

}

ops->set_vq_cb(vdpa, idx, &cb);

- vhost_vdpa_setup_vq_irq(v, idx);

break;

case VHOST_SET_VRING_NUM:

We can also track this issue in Bugzilla ticket 21171 
(https://bugzilla.kernel.org/show_bug.cgi?id=211711 
<https://bugzilla.kernel.org/show_bug.cgi?id=211711>) and the complete 
patch is attached with this email.
So vhost supports to remove or switch eventfd through 
vhost_vdpa_vring_ioctl(). So if userspace want to switch to another 
eventfd, we should re-do the register and unregister.

I think we need to deal this issue in another way. Can we check whether 
or not the producer is initialized before?

Thanks

Regards,

Gautam Dawar
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help