Re: Kernel panic with vhost-vdpa

Jason Wang <jasowang@xxxxxxxxxx> · Thu, 18 Feb 2021 16:48:18 +0800



    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.

    
        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) 
          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
      
    
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization