[Virtio-fs] [PATCH v2] virtiofsd: cancel all queue threads on exit in virtio_loop()

[Stefan Hajnoczi]

On Fri, Dec 20, 2019 at 11:12:13AM +0800, Eryu Guan wrote:
> On guest graceful shutdown, virtiofsd receives VHOST_USER_GET_VRING_BASE
> request from VMM and shuts down virtqueues by calling fv_set_started(),
> which joins fv_queue_thread() threads. So when virtio_loop() returns,
> there should be no thread is still accessing data in fuse session and/or
> virtio dev.
> 
> But on abnormal exit, e.g. guest got killed for whatever reason,
> vhost-user socket is closed and virtio_loop() breaks out the main loop
> and returns to main(). But it's possible fv_queue_worker()s are still
> working and accessing fuse session and virtio dev, which results in
> crash or use-after-free.
> 
> Fix it by cancelling fv_queue_thread()s before virtio_loop() returns,
> to make sure there's no-one could access fuse session and virtio dev.

Why use pthread_cancel(3) instead of writing 1 to the thread's kill_fd?

I'm concerned about pthread_cancel(3) because the threads access
rwlocks, which may be leaked when pthread_cancel(3) terminates the
thread.

It would be safer to reuse the following code to get full thread
cleanup:

  static void fv_queue_set_started(VuDev *dev, int qidx, bool started)
  {
        ...
        int ret;
        assert(qidx < vud->nqueues);
        ourqi = vud->qi[qidx];

        /* Kill the thread */
        if (eventfd_write(ourqi->kill_fd, 1)) {
            fuse_log(FUSE_LOG_ERR, "Eventfd_read for queue: %m\n");
        }
        ret = pthread_join(ourqi->thread, NULL);
        if (ret) {
            fuse_log(FUSE_LOG_ERR, "%s: Failed to join thread idx %d err %d\n",
                     __func__, qidx, ret);
        }
        pthread_mutex_destroy(&ourqi->vq_lock);
        close(ourqi->kill_fd);
        ourqi->kick_fd = -1;
        free(vud->qi[qidx]);
        vud->qi[qidx] = NULL;
	...
  }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virtio-fs/attachments/20200106/10a4c31a/attachment.sig>