linux-kernel - Re: [PATCH] include/linux/module.h: mark init/cleanup_module aliases as _

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:   Wed, 6 Feb 2019 17:31:24 +0100
From:   Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
To:     Jessica Yu <jeyu@...nel.org>
Cc:     Laura Abbott <labbott@...hat.com>,
        Martin Sebor <msebor@....gnu.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] include/linux/module.h: mark init/cleanup_module aliases
 as __cold

On Mon, Feb 4, 2019 at 4:08 PM Jessica Yu <jeyu@...nel.org> wrote:
>
> IMHO I think annotating with __init is more straightforward, instead
> of cherry-picking attributes (we wouldn't know at first glance why the
> aliases are specifically annotated with __cold without looking at git
> history). Plus the actual module init function and alias declarations
> would be consistent. Just looking at the __init attributes:
>
>     #define __init              __section(.init.text) __cold  __latent_entropy __noinitretpoline
>
> __section(.init.text) - alias already has same section ndx as the
> target symbol so this doesn't have any effect.
>
> __latent_entropy - according to commit 0766f788eb7, if this attribute
> is used on a function then the plugin will utilize it for gathering
> entropy (apparently a local variable is created in every marked
> function, the value of which is modified randomly, and before function
> return it will write into the latent_entropy global variable). Module
> init functions are already annotated with this since they are
> annotated with __init, I don't think marking the alias would do any
> harm.
>
> __noinitretpoline - compiled away if the function is in a module and
> not built-in. The alias is not utilized if the module is built-in. So
> this wouldn't apply to the alias.

In that case, there is also the option suggested by Martin: using the
new "copy" attribute which copies all attributes, except those
blacklisted by GCC, at the moment:

    alias, always_inline, gnu_inline, ifunc, noinline, visibility,
    weak, weakref

Since we have the __init macro, there is not much gain in this
instance (but if you prefer the copy alternative, let me know).

> Unfortunately I don't have gcc9 set up on my machine so I can't
> actually test if it gets rid of all the warnings, so testing this
> would be appreciated :)

The warning triggers currently for a subset of attributes only:

    alloc_align, alloc_size, cold, const, hot, leaf, malloc,
    nonnull, noreturn, nothrow, pure, returns_nonnull,
    returns_twice

So the rest of the attributes do not make a difference w.r.t. the warnings.

I will change it to __init then and send the PR after a couple of days
in -next :)

Cheers,
Miguel