Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1441693 > unrolled thread

Re: [PATCH v2 1/3] Add the initify gcc plugin

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2 1/3] Add the initify gcc plugin Kees Cook <keescook@chromium.org> - 2016-07-12 21:50 +0200
    Re: [PATCH v2 1/3] Add the initify gcc plugin Kees Cook <keescook@chromium.org> - 2016-07-12 22:10 +0200
      Re: [PATCH v2 1/3] Add the initify gcc plugin Emese Revfy <re.emese@gmail.com> - 2016-07-13 22:30 +0200
    Re: [PATCH v2 1/3] Add the initify gcc plugin Emese Revfy <re.emese@gmail.com> - 2016-07-12 22:10 +0200
    Re: [PATCH v2 1/3] Add the initify gcc plugin Russell King - ARM Linux <linux@armlinux.org.uk> - 2016-07-13 00:20 +0200
      Re: [PATCH v2 1/3] Add the initify gcc plugin Kees Cook <keescook@chromium.org> - 2016-07-13 00:40 +0200
        Re: [PATCH v2 1/3] Add the initify gcc plugin Emese Revfy <re.emese@gmail.com> - 2016-07-13 23:30 +0200
    Re: [PATCH v2 1/3] Add the initify gcc plugin Emese Revfy <re.emese@gmail.com> - 2016-07-13 22:50 +0200
      Re: [PATCH v2 1/3] Add the initify gcc plugin Kees Cook <keescook@chromium.org> - 2016-07-13 23:10 +0200

#1441693 — Re: [PATCH v2 1/3] Add the initify gcc plugin

On Mon, Jul 4, 2016 at 7:40 PM, Emese Revfy <re.emese@gmail.com> wrote:
> The kernel already has a mechanism to free up code and data memory that
> is only used during kernel or module initialization.
> This plugin will teach the compiler to find more such code and data that
> can be freed after initialization.
> It has two passes. The first one tries to find all functions that
> can be become __init/__exit. The second one moves string constants
> (local variables and function string arguments marked by
> the nocapture attribute) only referenced in __init/__exit functions
> to the __initconst/__exitconst sections.
> It reduces memory usage. This plugin can be useful for embedded systems.
>
> If a function is called by __init and __exit functions as well then
> the plugin moves it to the __exit section. This causes false positive
> section mismatch errors/warnings that I don't know how to handle yet.

Should the mismatch checker be updated to recognize this case? Without
the plugin, I assume these kinds of functions would only ever be
marked for __exit? If so, should the plugin strip the __init marking
and only add __exit?

> The instrumentation pass of the latent_entropy plugin must run after
> the initify plugin to increase coverage.
>
> Signed-off-by: Emese Revfy <re.emese@gmail.com>

Thanks for sending this! I'll get it added to my tree for some 0day
build testing, and then get it into my -next tree.

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [next] | [standalone]

#1441699

On Tue, Jul 12, 2016 at 4:07 PM, Emese Revfy <re.emese@gmail.com> wrote:
> On Tue, 12 Jul 2016 15:45:56 -0400
> Kees Cook <keescook@chromium.org> wrote:
>
>> On Mon, Jul 4, 2016 at 7:40 PM, Emese Revfy <re.emese@gmail.com> wrote:
>> Thanks for sending this! I'll get it added to my tree for some 0day
>> build testing, and then get it into my -next tree.
>
> I'll send a new patch set soon. I'm working on some fixes and
> I would like to implement the handling of return uses to increase the coverage
> (which decreases because of the bug fixes :) ).
> So please hold off with testing until the next patch.

Okay, cool. I'll just do some simple tests with what I have so I can
feel like I understand its intended use, etc. :)

One change I made was to add this comment in compiler-gcc.h for people
that become curious about __nocapture and go looking for its
definition (please feel free to adjust for accuracy, etc):


/*
 * The initify gcc-plugin attempts to identify const arguments that are only
 * used during init (see __init), so they can be moved to the .init.rodata
 * section. If an argument is passed to a non-init function, it must
 * normally be assumed that such an argument has been captured by that
 * function and may be used in the future when .init has been unmapped from
 * memory. In order to identify functions that are confirmed to not capture
 * their arguments, the __nocapture() attribute is used so that initify can
 * better identify candidate variables.
 */
#ifdef INITIFY_PLUGIN
# define __nocapture(...) __attribute__((nocapture(__VA_ARGS__)))
#endif


-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [prev] | [next] | [standalone]

#1442823

On Tue, 12 Jul 2016 16:05:45 -0400
Kees Cook <keescook@chromium.org> wrote:

> /*
>  * The initify gcc-plugin attempts to identify const arguments that are only
>  * used during init (see __init), so they can be moved to the .init.rodata
>  * section. If an argument is passed to a non-init function, it must
>  * normally be assumed that such an argument has been captured by that
>  * function and may be used in the future when .init has been unmapped from
>  * memory. In order to identify functions that are confirmed to not capture
>  * their arguments, the __nocapture() attribute is used so that initify can
>  * better identify candidate variables.
>  */
> #ifdef INITIFY_PLUGIN
> # define __nocapture(...) __attribute__((nocapture(__VA_ARGS__)))
> #endif

Thanks, I'll take it in the next patch set with some additions (the attribute
also handles __exit functions and the plugin does other things e.g., it can
identify candidate init/exit functions and move them automatically to init.text/exit.text).

-- 
Emese

[toc] | [prev] | [next] | [standalone]

#1441704

On Tue, 12 Jul 2016 15:45:56 -0400
Kees Cook <keescook@chromium.org> wrote:

> On Mon, Jul 4, 2016 at 7:40 PM, Emese Revfy <re.emese@gmail.com> wrote:
> Thanks for sending this! I'll get it added to my tree for some 0day
> build testing, and then get it into my -next tree.

I'll send a new patch set soon. I'm working on some fixes and
I would like to implement the handling of return uses to increase the coverage
(which decreases because of the bug fixes :) ).
So please hold off with testing until the next patch.

-- 
Emese

[toc] | [prev] | [next] | [standalone]

#1441793

On Tue, Jul 12, 2016 at 03:45:56PM -0400, Kees Cook wrote:
> On Mon, Jul 4, 2016 at 7:40 PM, Emese Revfy <re.emese@gmail.com> wrote:
> > If a function is called by __init and __exit functions as well then
> > the plugin moves it to the __exit section. This causes false positive
> > section mismatch errors/warnings that I don't know how to handle yet.
> 
> Should the mismatch checker be updated to recognize this case? Without
> the plugin, I assume these kinds of functions would only ever be
> marked for __exit? If so, should the plugin strip the __init marking
> and only add __exit?

That sounds like a problem for architectures that still discard the
__exit section at link time to reduce the size of the linked kernel
image - though, obviously, if using the plugin results in a smaller
kernel image _with_ the exit sections, then there's a net benefit
size-wise.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]

#1441815

On Tue, Jul 12, 2016 at 6:08 PM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Tue, Jul 12, 2016 at 03:45:56PM -0400, Kees Cook wrote:
>> On Mon, Jul 4, 2016 at 7:40 PM, Emese Revfy <re.emese@gmail.com> wrote:
>> > If a function is called by __init and __exit functions as well then
>> > the plugin moves it to the __exit section. This causes false positive
>> > section mismatch errors/warnings that I don't know how to handle yet.
>>
>> Should the mismatch checker be updated to recognize this case? Without
>> the plugin, I assume these kinds of functions would only ever be
>> marked for __exit? If so, should the plugin strip the __init marking
>> and only add __exit?
>
> That sounds like a problem for architectures that still discard the
> __exit section at link time to reduce the size of the linked kernel
> image - though, obviously, if using the plugin results in a smaller
> kernel image _with_ the exit sections, then there's a net benefit
> size-wise.

Ah right, __exit is dropped for non-modular builds. So, for "both
__init and __exit" it sounds like the behavior depends on the build:

- if modular: remove __init marking (since we need it after init)
- if non-modular: remove __exit marking (since we'll never call exit)

Is this something the build itself (rather than the plugin) could
notice and fix up? Hmmm

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [prev] | [next] | [standalone]

#1442872

On Tue, 12 Jul 2016 18:38:47 -0400
Kees Cook <keescook@chromium.org> wrote:

> On Tue, Jul 12, 2016 at 6:08 PM, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
> > On Tue, Jul 12, 2016 at 03:45:56PM -0400, Kees Cook wrote:
> >> On Mon, Jul 4, 2016 at 7:40 PM, Emese Revfy <re.emese@gmail.com> wrote:
> > That sounds like a problem for architectures that still discard the
> > __exit section at link time to reduce the size of the linked kernel
> > image - though, obviously, if using the plugin results in a smaller
> > kernel image _with_ the exit sections, then there's a net benefit
> > size-wise.
> 
> Ah right, __exit is dropped for non-modular builds. So, for "both
> __init and __exit" it sounds like the behavior depends on the build:
> 
> - if modular: remove __init marking (since we need it after init)
> - if non-modular: remove __exit marking (since we'll never call exit)

When gcc compiles vmlinux these functions should be in __init and when
it compiles *.ko then they can be in __exit. I have no time to do this
now but I added it to my todo list.

The temporary fix can be that I enable this section move only on x86
(on other archs it will decrase the coverage).

-- 
Emese

[toc] | [prev] | [next] | [standalone]

#1442838

On Tue, 12 Jul 2016 15:45:56 -0400
Kees Cook <keescook@chromium.org> wrote:

> On Mon, Jul 4, 2016 at 7:40 PM, Emese Revfy <re.emese@gmail.com> wrote:
> > If a function is called by __init and __exit functions as well then
> > the plugin moves it to the __exit section. This causes false positive
> > section mismatch errors/warnings that I don't know how to handle yet.
> 
> Should the mismatch checker be updated to recognize this case? Without
> the plugin, I assume these kinds of functions would only ever be
> marked for __exit? If so, should the plugin strip the __init marking
> and only add __exit?

I don't modify the existing attributes. I just add a new __init/__exit when
a function hasn't a section attribute yet.
There are three cases:
 * when the function is called only by __init functions then the plugin adds
   the __init attribute
 * when the function is called only by __exit functions then the plugin adds
   the __exit attribute
 * when the function is called by __init and __exit functions too then the
   plugin adds the __exit attribute.
The last case causes the false positive(?) message of the section mismatch.

-- 
Emese

[toc] | [prev] | [next] | [standalone]

#1442855

On Wed, Jul 13, 2016 at 1:48 PM, Emese Revfy <re.emese@gmail.com> wrote:
> On Tue, 12 Jul 2016 15:45:56 -0400
> Kees Cook <keescook@chromium.org> wrote:
>
>> On Mon, Jul 4, 2016 at 7:40 PM, Emese Revfy <re.emese@gmail.com> wrote:
>> > If a function is called by __init and __exit functions as well then
>> > the plugin moves it to the __exit section. This causes false positive
>> > section mismatch errors/warnings that I don't know how to handle yet.
>>
>> Should the mismatch checker be updated to recognize this case? Without
>> the plugin, I assume these kinds of functions would only ever be
>> marked for __exit? If so, should the plugin strip the __init marking
>> and only add __exit?
>
> I don't modify the existing attributes. I just add a new __init/__exit when
> a function hasn't a section attribute yet.
> There are three cases:
>  * when the function is called only by __init functions then the plugin adds
>    the __init attribute
>  * when the function is called only by __exit functions then the plugin adds
>    the __exit attribute
>  * when the function is called by __init and __exit functions too then the
>    plugin adds the __exit attribute.
> The last case causes the false positive(?) message of the section mismatch.

In the latter case, how does the linker actually choose where to put
such a function?

For a modular build, if it puts it in .init, it will be missing during
exit. If it puts it in .exit, that seems correct.

For a non-modular build, if it puts it in .init, this is correct. If
it puts it in .exit, it may be missing for init because the exit
section may have already been removed at final link time.

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web