linux-kernel - Re: [PATCH v4 19/27] x86: assembly, make some functions local

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 6 Oct 2017 14:21:21 +0100
From:   Mark Rutland <mark.rutland@....com>
To:     Jiri Slaby <jslaby@...e.cz>
Cc:     Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "hpa@...or.com" <hpa@...or.com>, "x86@...nel.org" <x86@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Matt Fleming <matt@...eblueprint.co.uk>,
        "linux-efi@...r.kernel.org" <linux-efi@...r.kernel.org>,
        xen-devel <xen-devel@...ts.xenproject.org>
Subject: Re: [PATCH v4 19/27] x86: assembly, make some functions local

Hi Jiri,

I can see that these serve a useful purpose (as they are necessary for
asm validation encessary for livepatching), and I am not personally
averse to the new annotations.

However, I am concerned that as-is, this is going to create more
problems for !x86 architectures. More on that below.

On Fri, Oct 06, 2017 at 02:53:08PM +0200, Jiri Slaby wrote:
> On 10/04/2017, 09:33 AM, Ard Biesheuvel wrote:
> > On 4 October 2017 at 08:22, Jiri Slaby <jslaby@...e.cz> wrote:
> >> On 10/02/2017, 02:48 PM, Ard Biesheuvel wrote:
> >>> On 2 October 2017 at 10:12, Jiri Slaby <jslaby@...e.cz> wrote:
> >>>> There is a couple of assembly functions, which are invoked only locally
> >>>> in the file they are defined. In C, we mark them "static". In assembly,
> >>>> annotate them using SYM_{FUNC,CODE}_START_LOCAL (and switch their
> >>>> ENDPROC to SYM_{FUNC,CODE}_END too). Whether FUNC or CODE depends on
> >>>> ENDPROC/END for a particular function (C or non-C).
> >>>
> >>> I wasn't cc'ed on the cover letter, so I am missing the rationale of
> >>> replacing ENTRY/ENDPROC with other macros.
> >>
> >> There was no cover letter. I am attaching what is in PATCH 1/27 instead:
> >> Introduce new C macros for annotations of functions and data in
> >> assembly. There is a long-standing mess in macros like ENTRY, END,
> >> ENDPROC and similar. They are used in different manners and sometimes
> >> incorrectly.
> > 
> > I must say, I don't share this sentiment.
> > 
> > In arm64, we use ENTRY/ENDPROC for functions with external linkage,
> > and the bare symbol name/ENDPROC for functions with local linkage. I
> > guess we could add ENDOBJECT if we wanted to, but we never really felt
> > the need.
> 
> Yes and this is exactly the reason for the new macros. Now, it's a
> complete mess. One arch does this, another does that. 

If the aim of this series is to introduce something that architectures
use consistently, then can we please actually poke other architectures
about it? e.g. send this to linux-arch, with a cover letter explaining
the idea and asking maintainers to take a look.

I think one reason that ENTRY/END/ENDPROC are used inconsistently is
that they're insufficiently documented. So people assume (inconsistent)
semantics, and often cargo-cult usage without thinking. To avoid that,
could we please document how these new macros should (and should not) be
used?

That way, we have a much better chance of consistency, and it's easier
to figure out if the intended semantics are necessary/sufficient.

Otherwise, I'm worried that bits of this will be inconsistently and
incorrectly cargo-culted into other architectures, making matters worse.

Thanks,
Mark.