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]

Message-ID: <CABCJKuc+mN4vw_sanZQKcb1=SyfT4h3JK2wpBuaB2qZH3Croxg@mail.gmail.com>
Date:   Fri, 15 Oct 2021 11:42:43 -0700
From:   Sami Tolvanen <samitolvanen@...gle.com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     Andy Lutomirski <luto@...nel.org>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        Kees Cook <keescook@...omium.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        "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 10:57 AM Thomas Gleixner <tglx@...utronix.de> wrote:
>
> On Fri, Oct 15 2021 at 17:55, Thomas Gleixner wrote:
> > On Thu, Oct 14 2021 at 19:51, Andy Lutomirski wrote:
> >> On Wed, Oct 13, 2021, at 11:16 AM, Sami Tolvanen wrote:
> > That still tells me:
> >
> >     1) This is a function
> >
> >     2) It has a regular argument which is expected to be in RDI
> >
> > which even allows to do analyis of e.g. the alternative call which
> > invokes that function.
> >
> > DECLARE_NOT_CALLED_FROM_C(clear_page_erms);
> >
> > loses these properties and IMO it's a tasteless hack.
>
> Look:
>
> SYSCALL_DEFINE2(set_robust_list, struct robust_list_head __user *, head,
>                 size_t, len)
>
> Not beautiful, but it gives the information which is needed and it tells
> me clearly what this is about. While the above lumps everything together
> whatever it is.

Sure, that makes sense. Ignoring the macro for a moment, how do you
feel about using incomplete structs for the non-C functions as Andy
suggested?

> Having __bikeshedme would allow to do:
>
>    __hardware_call
>    __xenhv_call
>    __inline_asm_call
>
> or such, which clearly tells how the function should be used and it can
> even be validated by tooling.

Previously you suggested adding a built-in function to the compiler:

https://lore.kernel.org/lkml/877dl0sc2m.ffs@nanos.tec.linutronix.de/

I actually did implement this in Clang, but the feature wasn't
necessary with opaque types, so I never moved forward with those
patches. A built-in also won't make the code any cleaner, which was a
concern last time.

I do agree that a function attribute would look cleaner, but it won't
stop anyone from mistakenly calling these functions from C code, which
was something Andy wanted to address at the same time. Do you still
prefer a function attribute over using an opaque type nevertheless?

> You could to that with macros as well, but thats not what you offered.
>
> Seriously, if you want to sell me that stuff, then you really should
> offer me something which has a value on its own and makes it palatable
> to me. That's not something new. See:
>
>   https://lore.kernel.org/lkml/alpine.LFD.2.00.1001251002430.3574@localhost.localdomain/
>
> That said, I still want to have a coherent technical explanation why the
> compiler people cannot come up with a sensible annotation for these
> things.

I can only assume they didn't think about this specific use case.

Sami