Skip to content

Commit a30badf

Browse files
dwmw2gregkh
authored andcommitted
hvc/xen: fix console unplug
On unplug of a Xen console, xencons_disconnect_backend() unconditionally calls free_irq() via unbind_from_irqhandler(), causing a warning of freeing an already-free IRQ: (qemu) device_del con1 [ 32.050919] ------------[ cut here ]------------ [ 32.050942] Trying to free already-free IRQ 33 [ 32.050990] WARNING: CPU: 0 PID: 51 at kernel/irq/manage.c:1895 __free_irq+0x1d4/0x330 It should be using evtchn_put() to tear down the event channel binding, and let the Linux IRQ side of it be handled by notifier_del_irq() through the HVC code. On which topic... xencons_disconnect_backend() should call hvc_remove() *first*, rather than tearing down the event channel and grant mapping while they are in use. And then the IRQ is guaranteed to be freed by the time it's torn down by evtchn_put(). Since evtchn_put() also closes the actual event channel, avoid calling xenbus_free_evtchn() except in the failure path where the IRQ was not successfully set up. However, calling hvc_remove() at the start of xencons_disconnect_backend() still isn't early enough. An unplug request is indicated by the backend setting its state to XenbusStateClosing, which triggers a notification to xencons_backend_changed(), which... does nothing except set its own frontend state directly to XenbusStateClosed without *actually* tearing down the HVC device or, you know, making sure it isn't actively in use. So the backend sees the guest frontend set its state to XenbusStateClosed and stops servicing the interrupt... and the guest spins for ever in the domU_write_console() function waiting for the ring to drain. Fix that one by calling hvc_remove() from xencons_backend_changed() before signalling to the backend that it's OK to proceed with the removal. Tested with 'dd if=/dev/zero of=/dev/hvc1' while telling Qemu to remove the console device. Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/20231020161529.355083-4-dwmw2@infradead.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 2704c9a commit a30badf

1 file changed

Lines changed: 24 additions & 8 deletions

File tree

drivers/tty/hvc/hvc_xen.c

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -377,18 +377,21 @@ void xen_console_resume(void)
377377
#ifdef CONFIG_HVC_XEN_FRONTEND
378378
static void xencons_disconnect_backend(struct xencons_info *info)
379379
{
380-
if (info->irq > 0)
381-
unbind_from_irqhandler(info->irq, NULL);
382-
info->irq = 0;
380+
if (info->hvc != NULL)
381+
hvc_remove(info->hvc);
382+
info->hvc = NULL;
383+
if (info->irq > 0) {
384+
evtchn_put(info->evtchn);
385+
info->irq = 0;
386+
info->evtchn = 0;
387+
}
388+
/* evtchn_put() will also close it so this is only an error path */
383389
if (info->evtchn > 0)
384390
xenbus_free_evtchn(info->xbdev, info->evtchn);
385391
info->evtchn = 0;
386392
if (info->gntref > 0)
387393
gnttab_free_grant_references(info->gntref);
388394
info->gntref = 0;
389-
if (info->hvc != NULL)
390-
hvc_remove(info->hvc);
391-
info->hvc = NULL;
392395
}
393396

394397
static void xencons_free(struct xencons_info *info)
@@ -553,10 +556,23 @@ static void xencons_backend_changed(struct xenbus_device *dev,
553556
if (dev->state == XenbusStateClosed)
554557
break;
555558
fallthrough; /* Missed the backend's CLOSING state */
556-
case XenbusStateClosing:
559+
case XenbusStateClosing: {
560+
struct xencons_info *info = dev_get_drvdata(&dev->dev);;
561+
562+
/*
563+
* Don't tear down the evtchn and grant ref before the other
564+
* end has disconnected, but do stop userspace from trying
565+
* to use the device before we allow the backend to close.
566+
*/
567+
if (info->hvc) {
568+
hvc_remove(info->hvc);
569+
info->hvc = NULL;
570+
}
571+
557572
xenbus_frontend_closed(dev);
558573
break;
559574
}
575+
}
560576
}
561577

562578
static const struct xenbus_device_id xencons_ids[] = {
@@ -616,7 +632,7 @@ static int __init xen_hvc_init(void)
616632
list_del(&info->list);
617633
spin_unlock_irqrestore(&xencons_lock, flags);
618634
if (info->irq)
619-
unbind_from_irqhandler(info->irq, NULL);
635+
evtchn_put(info->evtchn);
620636
kfree(info);
621637
return r;
622638
}

0 commit comments

Comments
 (0)