In this patch series I collected various improvements of the stackleak
gcc plugin.
The first patch excludes alloca() from the stackleak instrumentation logic
to make it simpler.
The second patch is the main improvement. It eliminates an unwanted
side-effect of kernel code instrumentation. This patch is a deep
reengineering of the idea described on grsecurity blog:
https://grsecurity.net/resolving_an_unfortunate_stackleak_interaction
The third patch adds 'verbose' plugin parameter for printing additional
info about the kernel code instrumentation.
Two other patches disable unneeded stackleak instrumentation for some
files.
I would like to thank Alexander Monakov <[email protected]> for his
advisory on gcc internals.
This patch series was tested for gcc version 4.8, 5, 6, 7, 8, 9, and 10
on x86_64, i386 and arm64.
That was done using the project 'kernel-build-containers':
https://github.com/a13xp0p0v/kernel-build-containers
Alexander Popov (5):
gcc-plugins/stackleak: Exclude alloca() from the instrumentation logic
gcc-plugins/stackleak: Use asm instrumentation to avoid useless
register saving
gcc-plugins/stackleak: Add 'verbose' plugin parameter
gcc-plugins/stackleak: Don't instrument itself
gcc-plugins/stackleak: Don't instrument vgettimeofday.c in arm64 VDSO
arch/arm64/kernel/vdso/Makefile | 3 +-
include/linux/compiler_attributes.h | 13 ++
kernel/Makefile | 1 +
kernel/stackleak.c | 16 +-
scripts/Makefile.gcc-plugins | 2 +
scripts/gcc-plugins/stackleak_plugin.c | 260 ++++++++++++++++++++-----
6 files changed, 232 insertions(+), 63 deletions(-)
--
2.25.2
Add 'verbose' plugin parameter for stackleak gcc plugin.
It can be used for printing additional info about the kernel code
instrumentation.
For using it add the following to scripts/Makefile.gcc-plugins:
gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STACKLEAK) \
+= -fplugin-arg-stackleak_plugin-verbose
Signed-off-by: Alexander Popov <[email protected]>
---
scripts/gcc-plugins/stackleak_plugin.c | 31 +++++++++++++++++++++-----
1 file changed, 26 insertions(+), 5 deletions(-)
diff --git a/scripts/gcc-plugins/stackleak_plugin.c b/scripts/gcc-plugins/stackleak_plugin.c
index 0769c5b9156d..19358712d4ed 100644
--- a/scripts/gcc-plugins/stackleak_plugin.c
+++ b/scripts/gcc-plugins/stackleak_plugin.c
@@ -33,6 +33,8 @@ __visible int plugin_is_GPL_compatible;
static int track_frame_size = -1;
static bool build_for_x86 = false;
static const char track_function[] = "stackleak_track_stack";
+static bool disable = false;
+static bool verbose = false;
/*
* Mark these global variables (roots) for gcc garbage collector since
@@ -45,6 +47,7 @@ static struct plugin_info stackleak_plugin_info = {
.help = "track-min-size=nn\ttrack stack for functions with a stack frame size >= nn bytes\n"
"arch=target_arch\tspecify target build arch\n"
"disable\t\tdo not activate the plugin\n"
+ "verbose\t\tprint info about the instrumentation\n"
};
static void add_stack_tracking_gcall(gimple_stmt_iterator *gsi)
@@ -98,6 +101,10 @@ static tree get_current_stack_pointer_decl(void)
return var;
}
+ if (verbose) {
+ fprintf(stderr, "stackleak: missing current_stack_pointer in %s()\n",
+ DECL_NAME_POINTER(current_function_decl));
+ }
return NULL_TREE;
}
@@ -366,6 +373,7 @@ static bool remove_stack_tracking_gasm(void)
*/
static unsigned int stackleak_cleanup_execute(void)
{
+ const char *fn = DECL_NAME_POINTER(current_function_decl);
bool removed = false;
/*
@@ -376,11 +384,17 @@ static unsigned int stackleak_cleanup_execute(void)
* For more info see gcc commit 7072df0aae0c59ae437e.
* Let's leave such functions instrumented.
*/
- if (cfun->calls_alloca)
+ if (cfun->calls_alloca) {
+ if (verbose)
+ fprintf(stderr, "stackleak: instrument %s() old\n", fn);
return 0;
+ }
- if (large_stack_frame())
+ if (large_stack_frame()) {
+ if (verbose)
+ fprintf(stderr, "stackleak: instrument %s()\n", fn);
return 0;
+ }
if (lookup_attribute_spec(get_identifier("no_caller_saved_registers")))
removed = remove_stack_tracking_gasm();
@@ -506,9 +520,6 @@ __visible int plugin_init(struct plugin_name_args *plugin_info,
/* Parse the plugin arguments */
for (i = 0; i < argc; i++) {
- if (!strcmp(argv[i].key, "disable"))
- return 0;
-
if (!strcmp(argv[i].key, "track-min-size")) {
if (!argv[i].value) {
error(G_("no value supplied for option '-fplugin-arg-%s-%s'"),
@@ -531,6 +542,10 @@ __visible int plugin_init(struct plugin_name_args *plugin_info,
if (!strcmp(argv[i].value, "x86"))
build_for_x86 = true;
+ } else if (!strcmp(argv[i].key, "disable")) {
+ disable = true;
+ } else if (!strcmp(argv[i].key, "verbose")) {
+ verbose = true;
} else {
error(G_("unknown option '-fplugin-arg-%s-%s'"),
plugin_name, argv[i].key);
@@ -538,6 +553,12 @@ __visible int plugin_init(struct plugin_name_args *plugin_info,
}
}
+ if (disable) {
+ if (verbose)
+ fprintf(stderr, "stackleak: disabled for this translation unit\n");
+ return 0;
+ }
+
/* Give the information about the plugin */
register_callback(plugin_name, PLUGIN_INFO, NULL,
&stackleak_plugin_info);
--
2.25.2
On Thu, Jun 04, 2020 at 04:49:52PM +0300, Alexander Popov wrote:
> In this patch series I collected various improvements of the stackleak
> gcc plugin.
Great; thank you! I'll take a closer look at this shortly!
--
Kees Cook
On Thu, Jun 04, 2020 at 04:49:55PM +0300, Alexander Popov wrote:
> Add 'verbose' plugin parameter for stackleak gcc plugin.
> It can be used for printing additional info about the kernel code
> instrumentation.
>
> For using it add the following to scripts/Makefile.gcc-plugins:
> gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STACKLEAK) \
> += -fplugin-arg-stackleak_plugin-verbose
>
> Signed-off-by: Alexander Popov <[email protected]>
Acked-by: Kees Cook <[email protected]>
--
Kees Cook
On Thu, Jun 04, 2020 at 04:49:52PM +0300, Alexander Popov wrote:
> In this patch series I collected various improvements of the stackleak
> gcc plugin.
Thanks!
> Alexander Popov (5):
> gcc-plugins/stackleak: Exclude alloca() from the instrumentation logic
> gcc-plugins/stackleak: Use asm instrumentation to avoid useless
> register saving
These look like they might need tweaks (noted in their separate
replies).
> gcc-plugins/stackleak: Add 'verbose' plugin parameter
> gcc-plugins/stackleak: Don't instrument itself
If you wanted to reorder the series and move these first, I could take
these into my tree right away (they're logically separate from the other
fixes).
> gcc-plugins/stackleak: Don't instrument vgettimeofday.c in arm64 VDSO
This seems good -- though I'm curious about 32-bit ARM and the other
HAVE_GCC_PLUGINS architectures with vDSOs (which appears to be all of
them except um).
--
Kees Cook
On 09.06.2020 22:15, Kees Cook wrote:
> On Thu, Jun 04, 2020 at 04:49:52PM +0300, Alexander Popov wrote:
>> In this patch series I collected various improvements of the stackleak
>> gcc plugin.
>
> Thanks!
>
>> Alexander Popov (5):
>> gcc-plugins/stackleak: Exclude alloca() from the instrumentation logic
>> gcc-plugins/stackleak: Use asm instrumentation to avoid useless
>> register saving
>
> These look like they might need tweaks (noted in their separate
> replies).
Thanks for the review, Kees.
>> gcc-plugins/stackleak: Add 'verbose' plugin parameter
>> gcc-plugins/stackleak: Don't instrument itself
>
> If you wanted to reorder the series and move these first, I could take
> these into my tree right away (they're logically separate from the other
> fixes).
Ok, I will put "don't instrument itself" at the beginning of v2.
The patch adding 'verbose' plugin parameter depends on the previous patches, so
I will not move it.
>> gcc-plugins/stackleak: Don't instrument vgettimeofday.c in arm64 VDSO
>
> This seems good -- though I'm curious about 32-bit ARM and the other
> HAVE_GCC_PLUGINS architectures with vDSOs (which appears to be all of
> them except um).
(going to reply in a separate email)
Best regards,
Alexander
On 09.06.2020 21:47, Kees Cook wrote:
> On Thu, Jun 04, 2020 at 04:49:55PM +0300, Alexander Popov wrote:
>> Add 'verbose' plugin parameter for stackleak gcc plugin.
>> It can be used for printing additional info about the kernel code
>> instrumentation.
>>
>> For using it add the following to scripts/Makefile.gcc-plugins:
>> gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STACKLEAK) \
>> += -fplugin-arg-stackleak_plugin-verbose
>>
>> Signed-off-by: Alexander Popov <[email protected]>
>
> Acked-by: Kees Cook <[email protected]>
I see that I will change this patch after leaving alloca() support.
I'm going to add debug printing about functions that call alloca().
I have to omit your 'acked-by' for the changed patch, right?
Best regards,
Alexander
On Wed, Jun 10, 2020 at 06:52:10PM +0300, Alexander Popov wrote:
> On 09.06.2020 21:47, Kees Cook wrote:
> > On Thu, Jun 04, 2020 at 04:49:55PM +0300, Alexander Popov wrote:
> >> Add 'verbose' plugin parameter for stackleak gcc plugin.
> >> It can be used for printing additional info about the kernel code
> >> instrumentation.
> >>
> >> For using it add the following to scripts/Makefile.gcc-plugins:
> >> gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STACKLEAK) \
> >> += -fplugin-arg-stackleak_plugin-verbose
> >>
> >> Signed-off-by: Alexander Popov <[email protected]>
> >
> > Acked-by: Kees Cook <[email protected]>
>
> I see that I will change this patch after leaving alloca() support.
> I'm going to add debug printing about functions that call alloca().
> I have to omit your 'acked-by' for the changed patch, right?
If it changes dramatically, drop my Ack, yes. But since it's going via
my tree, my Ack is mostly just a quick email marker to say "yes, looks
good". :)
--
Kees Cook