linux-hardening - Re: [RFC PATCH v2 07/21] cfi: Add type helper macros

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:   Mon, 16 May 2022 09:23:05 -0700
From:   Sami Tolvanen <samitolvanen@...gle.com>
To:     Rasmus Villemoes <linux@...musvillemoes.dk>
Cc:     Kees Cook <keescook@...omium.org>, linux-kernel@...r.kernel.org,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>, x86@...nel.org,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Joao Moreira <joao@...rdrivepizza.com>,
        Sedat Dilek <sedat.dilek@...il.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        linux-hardening@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, llvm@...ts.linux.dev
Subject: Re: [RFC PATCH v2 07/21] cfi: Add type helper macros

On Mon, May 16, 2022 at 5:28 AM Rasmus Villemoes
<linux@...musvillemoes.dk> wrote:
>
> On 14/05/2022 23.49, Kees Cook wrote:
> > On Fri, May 13, 2022 at 01:21:45PM -0700, Sami Tolvanen wrote:
> >> With CONFIG_CFI_CLANG, assembly functions called indirectly
> >> from C code must be annotated with type identifiers to pass CFI
> >> checking. The compiler emits a __kcfi_typeid_<function> symbol for
> >> each address-taken function declaration in C, which contains the
> >> expected type identifier. Add typed versions of SYM_FUNC_START and
> >> SYM_FUNC_START_ALIAS, which emit the type identifier before the
> >> function.
> >>
> >> Signed-off-by: Sami Tolvanen <samitolvanen@...gle.com>
> >
> > And the reason to not make this change universally (i.e. directly in
> > SYM_FUNC_START) is to minimize how many of these symbol annotations get
> > emitted? (And to more directly indicate which asm is called indirectly?)
> >
> > What happens if an asm function is called indirectly and it doesn't have
> > this annotation?
>
> Presumably that's a fail.
>
> I'm also interested in how this works at the asm/linker level. I assume
> that the .o file generated from the asm input has
> __kcfi_typeid_<function> as an undefined symbol; the compiler emits that
> symbol as an absolute one upon taking the address of <function>, and the
> linker then has the info it needs to patch things up.

Correct. The generated code looks like this:

00000000000003f7 <__cfi_blowfish_dec_blk>:
     3f7:       cc                      int3
     3f8:       cc                      int3
     3f9:       8b 04 25 00 00 00 00    mov    0x0,%eax
                        3fc: R_X86_64_32S       __kcfi_typeid_blowfish_dec_blk
     400:       cc                      int3
     401:       cc                      int3

0000000000000402 <blowfish_dec_blk>:

And the symbol table in the file that takes the address has this:

    45: ffffffffef478db5     0 NOTYPE  WEAK   DEFAULT  ABS
__kcfi_typeid_blowfish_dec_blk

> But what then happens if we have some function implemented in assembly
> which for whatever .config reason never has its address taken in any .c
> translation unit that gets linked in? Does the __kcfi_typeid_<function>
> symbol silently resolve to 0, or does the link fail?

It will fail to link in that case.

> I can't really imagine the compiler emitting __kcfi_typeid_<function>
> symbols for each and every function it sees merely declared in some header.

The compiler emits these only for address-taken declarations.

> Two different .c files both taking the address of <function> should of
> course emit the same value for __kcfi_typeid_<function>. Is there any
> sanity check anywhere that that's actually the case?

Not at the moment. I suppose we could warn about mismatches in the
linker though.

> Can we please have some objdump/readelf output from some .o files
> involved here?

Sure, I'll add examples to the commit message.

Sami