This is a note to let you know that I've just added the patch titled
hvc/xen: fix console unplug
to the 6.6-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
hvc-xen-fix-console-unplug.patch
and it can be found in the queue-6.6 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.
>From a30badfd7c13fc8763a9e10c5a12ba7f81515a55 Mon Sep 17 00:00:00 2001
From: David Woodhouse <dwmw@xxxxxxxxxxxx>
Date: Fri, 20 Oct 2023 17:15:29 +0100
Subject: hvc/xen: fix console unplug
From: David Woodhouse <dwmw@xxxxxxxxxxxx>
commit a30badfd7c13fc8763a9e10c5a12ba7f81515a55 upstream.
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@xxxxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
Link: https://lore.kernel.org/r/20231020161529.355083-4-dwmw2@xxxxxxxxxxxxx
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
drivers/tty/hvc/hvc_xen.c | 32 ++++++++++++++++++++++++--------
1 file changed, 24 insertions(+), 8 deletions(-)
--- a/drivers/tty/hvc/hvc_xen.c
+++ b/drivers/tty/hvc/hvc_xen.c
@@ -377,18 +377,21 @@ void xen_console_resume(void)
#ifdef CONFIG_HVC_XEN_FRONTEND
static void xencons_disconnect_backend(struct xencons_info *info)
{
- if (info->irq > 0)
- unbind_from_irqhandler(info->irq, NULL);
- info->irq = 0;
+ if (info->hvc != NULL)
+ hvc_remove(info->hvc);
+ info->hvc = NULL;
+ if (info->irq > 0) {
+ evtchn_put(info->evtchn);
+ info->irq = 0;
+ info->evtchn = 0;
+ }
+ /* evtchn_put() will also close it so this is only an error path */
if (info->evtchn > 0)
xenbus_free_evtchn(info->xbdev, info->evtchn);
info->evtchn = 0;
if (info->gntref > 0)
gnttab_free_grant_references(info->gntref);
info->gntref = 0;
- if (info->hvc != NULL)
- hvc_remove(info->hvc);
- info->hvc = NULL;
}
static void xencons_free(struct xencons_info *info)
@@ -553,10 +556,23 @@ static void xencons_backend_changed(stru
if (dev->state == XenbusStateClosed)
break;
fallthrough; /* Missed the backend's CLOSING state */
- case XenbusStateClosing:
+ case XenbusStateClosing: {
+ struct xencons_info *info = dev_get_drvdata(&dev->dev);;
+
+ /*
+ * Don't tear down the evtchn and grant ref before the other
+ * end has disconnected, but do stop userspace from trying
+ * to use the device before we allow the backend to close.
+ */
+ if (info->hvc) {
+ hvc_remove(info->hvc);
+ info->hvc = NULL;
+ }
+
xenbus_frontend_closed(dev);
break;
}
+ }
}
static const struct xenbus_device_id xencons_ids[] = {
@@ -616,7 +632,7 @@ static int __init xen_hvc_init(void)
list_del(&info->list);
spin_unlock_irqrestore(&xencons_lock, flags);
if (info->irq)
- unbind_from_irqhandler(info->irq, NULL);
+ evtchn_put(info->evtchn);
kfree(info);
return r;
}
Patches currently in stable-queue which might be from dwmw@xxxxxxxxxxxx are
queue-6.6/xen-events-avoid-using-info_for_irq-in-xen_send_ipi_.patch
queue-6.6/hvc-xen-fix-event-channel-handling-for-secondary-consoles.patch
queue-6.6/hvc-xen-fix-console-unplug.patch
queue-6.6/hvc-xen-fix-error-path-in-xen_hvc_init-to-always-register-frontend-driver.patch
