linux-kernel - Re: [PATCH v2 3/3] hvc/xen: fix console unplug

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <195567e6-a654-44f5-877c-0fc910f1f92b@suse.com>
Date:   Mon, 23 Oct 2023 10:14:32 +0200
From:   Juergen Gross <jgross@...e.com>
To:     David Woodhouse <dwmw2@...radead.org>,
        xen-devel@...ts.xenproject.org
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jirislaby@...nel.org>,
        Roger Pau Monne <roger.pau@...rix.com>,
        Stefano Stabellini <sstabellini@...nel.org>,
        Dawei Li <set_pte_at@...look.com>,
        linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
        linux-serial@...r.kernel.org, Paul Durrant <paul@....org>
Subject: Re: [PATCH v2 3/3] hvc/xen: fix console unplug

On 20.10.23 18:15, David Woodhouse wrote:
> From: David Woodhouse <dwmw@...zon.co.uk>
> 
> 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@...zon.co.uk>
> Cc: stable@...r.kernel.org

Reviewed-by: Juergen Gross <jgross@...e.com>


Juergen


Download attachment "OpenPGP_0xB0DE9DD628BF132F.asc" of type "application/pgp-keys" (3099 bytes)

Download attachment "OpenPGP_signature.asc" of type "application/pgp-signature" (496 bytes)