linux-kernel - Re: [PATCH v2 23/23] objtool, kcsan: Remove memory barrier instrumentation from noinstr

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: <CANpmjNPG1OdL9i73jiGH3XNmR+q+fRJfCaGrUXefRYu1kqhOGw@mail.gmail.com>
Date:   Fri, 19 Nov 2021 22:31:57 +0100
From:   Marco Elver <elver@...gle.com>
To:     Josh Poimboeuf <jpoimboe@...hat.com>
Cc:     "Paul E. McKenney" <paulmck@...nel.org>,
        Alexander Potapenko <glider@...gle.com>,
        Boqun Feng <boqun.feng@...il.com>,
        Borislav Petkov <bp@...en8.de>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Ingo Molnar <mingo@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Waiman Long <longman@...hat.com>,
        Will Deacon <will@...nel.org>, kasan-dev@...glegroups.com,
        linux-arch@...r.kernel.org, linux-doc@...r.kernel.org,
        linux-kbuild@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-mm@...ck.org, x86@...nel.org
Subject: Re: [PATCH v2 23/23] objtool, kcsan: Remove memory barrier
 instrumentation from noinstr

On Fri, 19 Nov 2021 at 21:31, Josh Poimboeuf <jpoimboe@...hat.com> wrote:

> > +     if (insn->sec->noinstr && sym->removable_instr) {
> >               if (reloc) {
> >                       reloc->type = R_NONE;
> >                       elf_write_reloc(file->elf, reloc);
>
> I'd love to have a clearer name than 'removable_instr', though I'm
> having trouble coming up with something.
>
> 'profiling_func'?
>
> Profiling isn't really accurate but maybe it gets the point across.  I'm
> definitely open to other suggestions.

Well, this bit is not true for all "profiling functions" either. It's
only true for instrumentation functions that appear in 'noinstr' and
that the compiler can't remove on its own, but are valid to remove by
objtool in noinstr code, hence 'removable_instr'.

I'm really quite indifferent what we call it, so I'll leave you to
pick whatever sounds best:

-- profiling_func
-- nop_profiling_func
-- optional_profiling_func
-- noinstr_remove
-- removable_profiling_func
-- noinstr_nop_func
-- noinstr_nop
-- nop_in_noinstr
-- invalid_in_noinstr

?

> Also, the above code isn't very self-evident so there still needs to be
> a comment there, like:
>
>         /*
>          * Many compilers cannot disable KCOV or sanitizer calls with a
>          * function attribute so they need a little help, NOP out any
>          * such calls from noinstr text.
>          */
>

I'll add it.

> > +{
> > +     /*
> > +      * Many compilers cannot disable KCOV with a function attribute so they
> > +      * need a little help, NOP out any KCOV calls from noinstr text.
> > +      */
> > +     if (!strncmp(name, "__sanitizer_cov_", 16))
> > +             return true;
>
> A comment is good here, but the NOP-ing bit seems out of place.

I'll fix that.