* [PATCH 1/1] stackleak: Register the 'stackleak_cleanup' pass before the 'mach' pass
@ 2018-11-30 13:19 Alexander Popov
2018-11-30 17:09 ` Kees Cook
0 siblings, 1 reply; 6+ messages in thread
From: Alexander Popov @ 2018-11-30 13:19 UTC (permalink / raw)
To: kernel-hardening, Kees Cook, Jann Horn, Andy Lutomirski,
Borislav Petkov, Thomas Gleixner, Dave Hansen, Steven Rostedt,
Peter Zijlstra, Masami Hiramatsu, Florian Weimer,
Richard Sandiford, Segher Boessenkool, Alexander Monakov,
Tycho Andersen, Laura Abbott, Mark Rutland, Emese Revfy,
Thomas Garnier, Ingo Molnar, Will Deacon, Alexei Starovoitov,
Ard Biesheuvel, H Peter Anvin, David S Miller, linux-arm-kernel,
gcc, alex.popov, linux-kernel
Currently the 'stackleak_cleanup' pass deleting a CALL insn is executed
after the 'reload' pass. That allows gcc to do some weird optimization in
function prologues and epilogues, which are generated later [1].
Let's avoid that by registering the 'stackleak_cleanup' pass before
the 'mach' pass, which performs the machine dependent code transformations.
It's the moment when the stack frame size is final and function prologues
and epilogues are already generated.
[1] https://www.openwall.com/lists/kernel-hardening/2018/11/23/2
Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Alexander Popov <alex.popov@linux.com>
---
scripts/gcc-plugins/stackleak_plugin.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/scripts/gcc-plugins/stackleak_plugin.c b/scripts/gcc-plugins/stackleak_plugin.c
index 2f48da9..6f41b32 100644
--- a/scripts/gcc-plugins/stackleak_plugin.c
+++ b/scripts/gcc-plugins/stackleak_plugin.c
@@ -363,10 +363,12 @@ __visible int plugin_init(struct plugin_name_args *plugin_info,
PASS_POS_INSERT_BEFORE);
/*
- * The stackleak_cleanup pass should be executed after the
- * "reload" pass, when the stack frame size is final.
+ * The stackleak_cleanup pass should be executed before the "mach"
+ * pass, which performs the machine dependent code transformations.
+ * It's the moment when the stack frame size is already final and
+ * function prologues and epilogues are generated.
*/
- PASS_INFO(stackleak_cleanup, "reload", 1, PASS_POS_INSERT_AFTER);
+ PASS_INFO(stackleak_cleanup, "mach", 1, PASS_POS_INSERT_BEFORE);
if (!plugin_default_version_check(version, &gcc_version)) {
error(G_("incompatible gcc/plugin versions"));
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] stackleak: Register the 'stackleak_cleanup' pass before the 'mach' pass
2018-11-30 13:19 [PATCH 1/1] stackleak: Register the 'stackleak_cleanup' pass before the 'mach' pass Alexander Popov
@ 2018-11-30 17:09 ` Kees Cook
2018-11-30 17:12 ` Kees Cook
0 siblings, 1 reply; 6+ messages in thread
From: Kees Cook @ 2018-11-30 17:09 UTC (permalink / raw)
To: Alexander Popov
Cc: Mark Rutland, Kernel Hardening, Peter Zijlstra, Richard Sandiford,
Dave Hansen, Will Deacon, Alexei Starovoitov, H. Peter Anvin,
Ingo Molnar, Tycho Andersen, Emese Revfy, Laura Abbott,
Segher Boessenkool, Jann Horn, amonakov, Steven Rostedt,
Borislav Petkov, Andy Lutomirski, Thomas Gleixner,
linux-arm-kernel, Florian Weimer, gcc, Ard Biesheuvel, LKML,
David S. Miller, Masami Hiramatsu, Thomas Garnier
On Fri, Nov 30, 2018 at 5:20 AM Alexander Popov <alex.popov@linux.com> wrote:
>
> Currently the 'stackleak_cleanup' pass deleting a CALL insn is executed
> after the 'reload' pass. That allows gcc to do some weird optimization in
> function prologues and epilogues, which are generated later [1].
>
> Let's avoid that by registering the 'stackleak_cleanup' pass before
> the 'mach' pass, which performs the machine dependent code transformations.
> It's the moment when the stack frame size is final and function prologues
> and epilogues are already generated.
>
> [1] https://www.openwall.com/lists/kernel-hardening/2018/11/23/2
>
> Reported-by: kbuild test robot <lkp@intel.com>
> Signed-off-by: Alexander Popov <alex.popov@linux.com>
Thanks, applied!
-Kees
> ---
> scripts/gcc-plugins/stackleak_plugin.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/gcc-plugins/stackleak_plugin.c b/scripts/gcc-plugins/stackleak_plugin.c
> index 2f48da9..6f41b32 100644
> --- a/scripts/gcc-plugins/stackleak_plugin.c
> +++ b/scripts/gcc-plugins/stackleak_plugin.c
> @@ -363,10 +363,12 @@ __visible int plugin_init(struct plugin_name_args *plugin_info,
> PASS_POS_INSERT_BEFORE);
>
> /*
> - * The stackleak_cleanup pass should be executed after the
> - * "reload" pass, when the stack frame size is final.
> + * The stackleak_cleanup pass should be executed before the "mach"
> + * pass, which performs the machine dependent code transformations.
> + * It's the moment when the stack frame size is already final and
> + * function prologues and epilogues are generated.
> */
> - PASS_INFO(stackleak_cleanup, "reload", 1, PASS_POS_INSERT_AFTER);
> + PASS_INFO(stackleak_cleanup, "mach", 1, PASS_POS_INSERT_BEFORE);
>
> if (!plugin_default_version_check(version, &gcc_version)) {
> error(G_("incompatible gcc/plugin versions"));
> --
> 2.7.4
>
--
Kees Cook
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] stackleak: Register the 'stackleak_cleanup' pass before the 'mach' pass
2018-11-30 17:09 ` Kees Cook
@ 2018-11-30 17:12 ` Kees Cook
2018-11-30 22:01 ` Alexander Popov
2018-12-03 18:25 ` Alexander Popov
0 siblings, 2 replies; 6+ messages in thread
From: Kees Cook @ 2018-11-30 17:12 UTC (permalink / raw)
To: Alexander Popov
Cc: Mark Rutland, Kernel Hardening, Peter Zijlstra, Richard Sandiford,
Dave Hansen, Will Deacon, Alexei Starovoitov, H. Peter Anvin,
Ingo Molnar, Tycho Andersen, Emese Revfy, Laura Abbott,
Segher Boessenkool, Jann Horn, amonakov, Steven Rostedt,
Borislav Petkov, Andy Lutomirski, Thomas Gleixner,
linux-arm-kernel, Florian Weimer, gcc, Ard Biesheuvel, LKML,
David S. Miller, Masami Hiramatsu, Thomas Garnier
On Fri, Nov 30, 2018 at 9:09 AM Kees Cook <keescook@chromium.org> wrote:
>
> On Fri, Nov 30, 2018 at 5:20 AM Alexander Popov <alex.popov@linux.com> wrote:
> >
> > Currently the 'stackleak_cleanup' pass deleting a CALL insn is executed
> > after the 'reload' pass. That allows gcc to do some weird optimization in
> > function prologues and epilogues, which are generated later [1].
> >
> > Let's avoid that by registering the 'stackleak_cleanup' pass before
> > the 'mach' pass, which performs the machine dependent code transformations.
> > It's the moment when the stack frame size is final and function prologues
> > and epilogues are already generated.
> >
> > [1] https://www.openwall.com/lists/kernel-hardening/2018/11/23/2
> >
> > Reported-by: kbuild test robot <lkp@intel.com>
> > Signed-off-by: Alexander Popov <alex.popov@linux.com>
>
> Thanks, applied!
Eek, no, this is breaking my build badly:
*** WARNING *** there are active plugins, do not report this as a bug
unless you can reproduce it without enabling any plugins.
Event | Plugins
PLUGIN_START_UNIT | stackleak_plugin
kernel/exit.c: In function ‘release_task’:
kernel/exit.c:228:1: internal compiler error: Segmentation fault
}
Failing with:
gcc (Ubuntu 7.3.0-16ubuntu3) 7.3.0
-Kees
>
> -Kees
>
> > ---
> > scripts/gcc-plugins/stackleak_plugin.c | 8 +++++---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/scripts/gcc-plugins/stackleak_plugin.c b/scripts/gcc-plugins/stackleak_plugin.c
> > index 2f48da9..6f41b32 100644
> > --- a/scripts/gcc-plugins/stackleak_plugin.c
> > +++ b/scripts/gcc-plugins/stackleak_plugin.c
> > @@ -363,10 +363,12 @@ __visible int plugin_init(struct plugin_name_args *plugin_info,
> > PASS_POS_INSERT_BEFORE);
> >
> > /*
> > - * The stackleak_cleanup pass should be executed after the
> > - * "reload" pass, when the stack frame size is final.
> > + * The stackleak_cleanup pass should be executed before the "mach"
> > + * pass, which performs the machine dependent code transformations.
> > + * It's the moment when the stack frame size is already final and
> > + * function prologues and epilogues are generated.
> > */
> > - PASS_INFO(stackleak_cleanup, "reload", 1, PASS_POS_INSERT_AFTER);
> > + PASS_INFO(stackleak_cleanup, "mach", 1, PASS_POS_INSERT_BEFORE);
> >
> > if (!plugin_default_version_check(version, &gcc_version)) {
> > error(G_("incompatible gcc/plugin versions"));
> > --
> > 2.7.4
> >
>
>
> --
> Kees Cook
--
Kees Cook
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] stackleak: Register the 'stackleak_cleanup' pass before the 'mach' pass
2018-11-30 17:12 ` Kees Cook
@ 2018-11-30 22:01 ` Alexander Popov
2018-12-03 18:25 ` Alexander Popov
1 sibling, 0 replies; 6+ messages in thread
From: Alexander Popov @ 2018-11-30 22:01 UTC (permalink / raw)
To: Kees Cook
Cc: Mark Rutland, Kernel Hardening, Peter Zijlstra, Richard Sandiford,
Dave Hansen, Will Deacon, Alexei Starovoitov, H. Peter Anvin,
Ingo Molnar, Tycho Andersen, Emese Revfy, Laura Abbott,
Segher Boessenkool, Jann Horn, amonakov, Steven Rostedt,
Borislav Petkov, Andy Lutomirski, Thomas Gleixner,
linux-arm-kernel, Florian Weimer, gcc, Ard Biesheuvel, LKML,
David S. Miller, Masami Hiramatsu, Thomas Garnier
On 30.11.2018 20:12, Kees Cook wrote:
> On Fri, Nov 30, 2018 at 9:09 AM Kees Cook <keescook@chromium.org> wrote:
>>
>> On Fri, Nov 30, 2018 at 5:20 AM Alexander Popov <alex.popov@linux.com> wrote:
>>>
>>> Currently the 'stackleak_cleanup' pass deleting a CALL insn is executed
>>> after the 'reload' pass. That allows gcc to do some weird optimization in
>>> function prologues and epilogues, which are generated later [1].
>>>
>>> Let's avoid that by registering the 'stackleak_cleanup' pass before
>>> the 'mach' pass, which performs the machine dependent code transformations.
>>> It's the moment when the stack frame size is final and function prologues
>>> and epilogues are already generated.
>>>
>>> [1] https://www.openwall.com/lists/kernel-hardening/2018/11/23/2
>>>
>>> Reported-by: kbuild test robot <lkp@intel.com>
>>> Signed-off-by: Alexander Popov <alex.popov@linux.com>
>>
>> Thanks, applied!
>
> Eek, no, this is breaking my build badly:
>
> *** WARNING *** there are active plugins, do not report this as a bug
> unless you can reproduce it without enabling any plugins.
> Event | Plugins
> PLUGIN_START_UNIT | stackleak_plugin
> kernel/exit.c: In function ‘release_task’:
> kernel/exit.c:228:1: internal compiler error: Segmentation fault
> }
>
> Failing with:
>
> gcc (Ubuntu 7.3.0-16ubuntu3) 7.3.0
Nice... I don't reproduce it with gcc-7.3 built from source.
I'll investigate this, return with details and we'll decide what to do.
Thanks,
Alexander
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] stackleak: Register the 'stackleak_cleanup' pass before the 'mach' pass
2018-11-30 17:12 ` Kees Cook
2018-11-30 22:01 ` Alexander Popov
@ 2018-12-03 18:25 ` Alexander Popov
2018-12-06 15:10 ` Alexander Popov
1 sibling, 1 reply; 6+ messages in thread
From: Alexander Popov @ 2018-12-03 18:25 UTC (permalink / raw)
To: Kees Cook
Cc: Mark Rutland, Kernel Hardening, Peter Zijlstra, Richard Sandiford,
Dave Hansen, Will Deacon, Alexei Starovoitov, H. Peter Anvin,
Ingo Molnar, Tycho Andersen, Emese Revfy, Laura Abbott,
Segher Boessenkool, Jann Horn, amonakov, Steven Rostedt,
Borislav Petkov, Andy Lutomirski, Thomas Gleixner,
linux-arm-kernel, Florian Weimer, gcc, Ard Biesheuvel, LKML,
David S. Miller, Masami Hiramatsu, Thomas Garnier
On 30.11.2018 20:12, Kees Cook wrote:
> On Fri, Nov 30, 2018 at 9:09 AM Kees Cook <keescook@chromium.org> wrote:
>>
>> On Fri, Nov 30, 2018 at 5:20 AM Alexander Popov <alex.popov@linux.com> wrote:
>>>
>>> Currently the 'stackleak_cleanup' pass deleting a CALL insn is executed
>>> after the 'reload' pass. That allows gcc to do some weird optimization in
>>> function prologues and epilogues, which are generated later [1].
>>>
>>> Let's avoid that by registering the 'stackleak_cleanup' pass before
>>> the 'mach' pass, which performs the machine dependent code transformations.
>>> It's the moment when the stack frame size is final and function prologues
>>> and epilogues are already generated.
>>>
>>> [1] https://www.openwall.com/lists/kernel-hardening/2018/11/23/2
>>>
>>> Reported-by: kbuild test robot <lkp@intel.com>
>>> Signed-off-by: Alexander Popov <alex.popov@linux.com>
>>
>> Thanks, applied!
>
> Eek, no, this is breaking my build badly:
>
> *** WARNING *** there are active plugins, do not report this as a bug
> unless you can reproduce it without enabling any plugins.
> Event | Plugins
> PLUGIN_START_UNIT | stackleak_plugin
> kernel/exit.c: In function ‘release_task’:
> kernel/exit.c:228:1: internal compiler error: Segmentation fault
> }
>
> Failing with:
>
> gcc (Ubuntu 7.3.0-16ubuntu3) 7.3.0
I've done debugging of gcc with gdb and now understand my mistake.
It turned out that I register the 'stackleak_cleanup' pass deleting CALL insn
for that particular moment when the control flow graph is inconsistent.
That's what the machine-specific reorg passes do on various architectures:
/* We are freeing block_for_insn in the toplev to keep compatibility
with old MDEP_REORGS that are not CFG based. Recompute it now. */
compute_bb_for_insn ();
So recomputing basic block info for insns before calling delete_insn_and_edges()
fixes the issue.
But I think it's better to register the 'stackleak_cleanup' pass just one pass
earlier -- before the '*free_cfg' pass. I'll double check it for different
versions of gcc on all supported architectures and return with a new patch.
Best regards,
Alexander
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] stackleak: Register the 'stackleak_cleanup' pass before the 'mach' pass
2018-12-03 18:25 ` Alexander Popov
@ 2018-12-06 15:10 ` Alexander Popov
0 siblings, 0 replies; 6+ messages in thread
From: Alexander Popov @ 2018-12-06 15:10 UTC (permalink / raw)
To: Kees Cook
Cc: Mark Rutland, Kernel Hardening, Peter Zijlstra, Richard Sandiford,
Dave Hansen, Will Deacon, Alexei Starovoitov, H. Peter Anvin,
Ingo Molnar, Tycho Andersen, Emese Revfy, Laura Abbott,
Segher Boessenkool, Jann Horn, amonakov, Steven Rostedt,
Borislav Petkov, Andy Lutomirski, Thomas Gleixner,
linux-arm-kernel, Florian Weimer, gcc, Ard Biesheuvel, LKML,
David S. Miller, Masami Hiramatsu, Thomas Garnier
On 03.12.2018 21:25, Alexander Popov wrote:
> But I think it's better to register the 'stackleak_cleanup' pass just one pass
> earlier -- before the '*free_cfg' pass. I'll double check it for different
> versions of gcc on all supported architectures and return with a new patch.
I've tested this idea for gcc-5,6,7,8 on x86_64, x86_32, and arm64.
I'll send the patch soon.
Best regards,
Alexander
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-12-06 15:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-30 13:19 [PATCH 1/1] stackleak: Register the 'stackleak_cleanup' pass before the 'mach' pass Alexander Popov
2018-11-30 17:09 ` Kees Cook
2018-11-30 17:12 ` Kees Cook
2018-11-30 22:01 ` Alexander Popov
2018-12-03 18:25 ` Alexander Popov
2018-12-06 15:10 ` Alexander Popov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox