linux-kernel - Re: [PATCH v5 03/15] linkage: Add DECLARE_NOT_CALLED_FROM

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]

Date:   Sat, 16 Oct 2021 14:12:00 -0700
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Sami Tolvanen <samitolvanen@...gle.com>
Cc:     Andy Lutomirski <luto@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        the arch/x86 maintainers <x86@...nel.org>,
        Kees Cook <keescook@...omium.org>,
        "Peter Zijlstra (Intel)" <peterz@...radead.org>,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Sedat Dilek <sedat.dilek@...il.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        linux-hardening@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        llvm@...ts.linux.dev
Subject: Re: [PATCH v5 03/15] linkage: Add DECLARE_NOT_CALLED_FROM_C

On Fri, Oct 15, 2021 at 01:37:02PM -0700, Sami Tolvanen wrote:
> > But we *also* have the read-the-address thing:
> >
> > void something(void)
> > {
> >   /* actual C body */
> > }
> > alternative_call(something, someotherthing, ...);
> >
> > That wants to expand to assembly code that does:
> >
> > CALL [target]
> >
> > where [target] is the actual first instruction of real code and not
> > a CFI prologue.
> 
> Yes, here we would ideally want to avoid the CFI stub for better
> performance, but nothing actually breaks even if we don't.

That's because there's no CFI involved in alternative_call().  It
doesn't use function pointers.  It uses direct calls.

So all the discussion about clear_page_*() is completely irrelevant.  It
has nothing to do with CFI.

Same for copy_user_enhanced_fast_string() and friends.

> > And this all wants to work both for asm-defined functions and
> > C-defined functions.  This really is orthogonal to the
> > is-it-asm-or-is-it-C things.  All four combinations are possible.
> >
> > Does this make any sense?

Not really, I think Sami debunked most of your theories :-)

I think you're misunderstanding how Clang CFI works.  It doesn't
instrument the target function.  It hooks into the caller's function
pointer relocation, so when I try to call func_ptr(), the compiler
hijacks the function pointer and forces me to call into a
func_ptr.cfi_jt() checking function instead.

> > I kind of thing we want the attributes and the builtin, along the lines of:
> >
> > asm("call %m", function_nocfi_address(something));
> >
> > or however else we wire it up.
> >
> > (And, of course, the things that aren't C functions at all, like
> > exception entries, should be opaque.)
> 
> I agree, there are cases where having a function attribute and/or a
> built-in to stop the compiler from interfering would be useful. I'll
> dust off my patch series and see how the LLVM folks feel about it.

With all the talk about function attributes I still haven't heard a
clear and specific case where one is needed.

If you take out the things that don't actually need the
DEFINE_NOT_CALLED_FROM_C() annotation then all you have left is the need
for opaque structs as far as I can tell.

-- 
Josh