linux-kernel - Re: xen/evtchn and forced threaded irq

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <21214d47-9c68-e6bf-007a-4047cc2b02f9@oracle.com>
Date:   Wed, 20 Feb 2019 12:07:26 -0500
From:   Boris Ostrovsky <boris.ostrovsky@...cle.com>
To:     Julien Grall <julien.grall@....com>
Cc:     Juergen Gross <jgross@...e.com>,
        Stefano Stabellini <sstabellini@...nel.org>,
        xen-devel <xen-devel@...ts.xenproject.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Dave P Martin <dave.martin@....com>
Subject: Re: xen/evtchn and forced threaded irq

On 2/20/19 9:15 AM, Julien Grall wrote:
> Hi Boris,
>
> Thank you for your answer.
>
> On 20/02/2019 00:02, Boris Ostrovsky wrote:
>> On Tue, Feb 19, 2019 at 05:31:10PM +0000, Julien Grall wrote:
>>> Hi all,
>>>
>>> I have been looking at using Linux RT in Dom0. Once the guest is
>>> started,
>>> the console is ending to have a lot of warning (see trace below).
>>>
>>> After some investigation, this is because the irq handler will now
>>> be threaded.
>>> I can reproduce the same error with the vanilla Linux when passing
>>> the option
>>> 'threadirqs' on the command line (the trace below is from 5.0.0-rc7
>>> that has
>>> not RT support).
>>>
>>> FWIW, the interrupt for port 6 is used to for the guest to
>>> communicate with
>>> xenstore.
>>>
>>>  From my understanding, this is happening because the interrupt
>>> handler is now
>>> run in a thread. So we can have the following happening.
>>>
>>>     Interrupt context            |     Interrupt thread
>>>                                  |
>>>     receive interrupt port 6     |
>>>     clear the evtchn port        |
>>>     set IRQF_RUNTHREAD            |
>>>     kick interrupt thread        |
>>>                                  |    clear IRQF_RUNTHREAD
>>>                                  |    call evtchn_interrupt
>>>     receive interrupt port 6     |
>>>     clear the evtchn port        |
>>>     set IRQF_RUNTHREAD           |
>>>     kick interrupt thread        |
>>>                                  |    disable interrupt port 6
>>>                                  |    evtchn->enabled = false
>>>                                  |    [....]
>>>                                  |
>>>                                  |    *** Handling the second
>>> interrupt ***
>>>                                  |    clear IRQF_RUNTHREAD
>>>                                  |    call evtchn_interrupt
>>>                                  |    WARN(...)
>>>
>>> I am not entirely sure how to fix this. I have two solutions in mind:
>>>
>>> 1) Prevent the interrupt handler to be threaded. We would also need to
>>> switch from spin_lock to raw_spin_lock as the former may sleep on
>>> RT-Linux.
>>>
>>> 2) Remove the warning
>>
>> I think access to evtchn->enabled is racy so (with or without the
>> warning) we can't use it reliably.
>
> Thinking about it, it would not be the only issue. The ring is sized
> to contain only one instance of the same event. So if you receive
> twice the event, you may overflow the ring.

Hm... That's another argument in favor of "unthreading" the handler.

>
>>
>> Another alternative could be to queue the irq if !evtchn->enabled and
>> handle it in evtchn_write() (which is where irq is supposed to be
>> re-enabled).
> What do you mean by queue? Is it queueing in the ring? 


No, I was thinking about having a new structure for deferred interrupts.

-boris

> If so, wouldn't it be racy as described above?