linux-kernel - Re: [PATCH] kcov: properly check if we are in an interrupt

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: <CACT4Y+ZRrtB6h5+96WYVhVugvTVY+q9Aaef2AH_gc3fNn1GzdA@mail.gmail.com>
Date:   Tue, 27 Sep 2016 14:32:17 +0200
From:   Dmitry Vyukov <dvyukov@...gle.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Andrey Konovalov <andreyknvl@...gle.com>,
        Nicolai Stange <nicstange@...il.com>,
        Andrey Ryabinin <aryabinin@...tuozzo.com>,
        Kees Cook <keescook@...omium.org>,
        James Morse <james.morse@....com>,
        LKML <linux-kernel@...r.kernel.org>,
        Vegard Nossum <vegard.nossum@...cle.com>,
        Quentin Casasnovas <quentin.casasnovas@...cle.com>,
        Andrey Ryabinin <ryabinin.a.a@...il.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...e.hu>
Subject: Re: [PATCH] kcov: properly check if we are in an interrupt

On Tue, Sep 27, 2016 at 12:59 PM, Peter Zijlstra <peterz@...radead.org> wrote:
> On Tue, Sep 27, 2016 at 09:50:41AM +0200, Dmitry Vyukov wrote:
>> On Tue, Sep 27, 2016 at 9:34 AM, Peter Zijlstra <peterz@...radead.org> wrote:
>> > On Tue, Sep 27, 2016 at 08:21:32AM +0200, Dmitry Vyukov wrote:
>> >>
>> >> I suspect there is a bunch of places that use in_interrupt(), but mean
>> >> the same as KCOV wants -- am I in interrupt? and not am I in interrupt
>> >> context or in normal task context but inside local_bh_disable(). For
>> >> example, why does fput handles closure asynchronously if the task
>> >> called local_bh_disable?
>> >
>> > Agreed, but it would mean auditing all in_interrupt()/irq_count() users.
>>
>>
>> I don't think this means auditing all users. We are not making things
>> worse by introduction of a new predicate.
>> It would be nice to look at some uses in core code, but the only place
>> with observed harm is KCOV.
>>
>> Any naming suggestions? Other than really_in_interrupt or
>> in_interrupt_and_not_in_bh_disabled?
>
> Hence the suggestion to audit and fix instead of making a bigger mess :/


Ah, I see what you mean.
I am not sure Andrey and me are well equipped to tackle it. We have
very narrow kernel experience. Lots of uses are in drivers, which we
don't even know how to invoke. Guessing re current state of the
things, calling potentially expensive functions with bh disabled is
probably bad, but we have no idea as to the threshold.

Any ideas regarding a shorter term fix? We have a very real problem in
KCOV. We could open code and comment the new check in KCOV. Or maybe
do a global s/in_interrupt/in_interrupt_or_bh_disabled/ and then
introduce new in_interrupt. I don't know what's the practice with such
global changes.