linux-kernel - Re: Problem with commit bf22ff45bed664aefb5c4e43029057a199b7070c

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 for Android: free password hash cracker in your pocket

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

Date:   Fri, 7 Jul 2017 19:11:33 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Juergen Gross <jgross@...e.com>
cc:     jeffy.chen@...k-chips.com,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        xen-devel <xen-devel@...ts.xenproject.org>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
        Marc Zyngier <marc.zyngier@....com>,
        Peter Zijlstra <peterz@...radead.org>
Subject: Re: Problem with commit bf22ff45bed664aefb5c4e43029057a199b7070c

On Fri, 7 Jul 2017, Thomas Gleixner wrote:

> On Fri, 7 Jul 2017, Juergen Gross wrote:
> 
> > Commit bf22ff45bed664aefb5c4e43029057a199b7070c ("genirq: Avoid
> > unnecessary low level irq function calls") breaks Xen guest
> > save/restore handling.
> > 
> > The main problem are the PV devices using Xen event channels as
> > interrupt sources which are represented as an "irq chip" in the kernel.
> > When saving the guest the event channels are masked internally. At
> > restore time event channels are re-established and unmasked via
> > irq_startup().

And how exactly gets irq_startup() invoked on those event channels?

> > I have a patch repairing the issue, but I'm not sure if this way to do
> > it would be accepted. I have exported mask_irq() and I'm doing the
> > masking now through this function. Would the attached patch be
> > acceptable? Or is there a better way to solve the problem?
> 
> Without looking at the patch (too lazy to fiddle with attachments right
> now), this is definitely wrong. I'll have a look later tonight.

Not that I'm surprised, but that patch is exactly what I expected. Export a
random function, which helps to paper over the real problem and run away.
These functions are internal for a reason and we worked hard on making
people understand that fiddling with the internals of interrupts is a
NONO. If there are special requirements for a good reason, then we create
proper interfaces and infrastructure, if there is no good reason, then the
problematic code needs to be fixed. There is no exception for XEN.

Can you please explain how that save/restore stuff works and which
functions are involved?

Thanks,

	tglx