Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sun, 16 Apr 2017 22:30:52 -0700
From: Jessica Yu <jeyu@...hat.com>
To: Kees Cook <keescook@...omium.org>
Cc: Michael Leibowitz <michael.leibowitz@...el.com>,
	"kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>
Subject: Re: [PATCH 01/18] gcc-plugins: Add the randstruct plugin

+++ Kees Cook [12/04/17 15:12 -0700]:
>On Thu, Apr 6, 2017 at 2:18 PM, Kees Cook <keescook@...omium.org> wrote:
>> From: Michael Leibowitz <michael.leibowitz@...el.com>
>>
>> This plugin randomizes the layout of selected structures at compile
>> time. This is a probabilistic defense against attacks that need to
>> know the layout of structures within the kernel. While less useful for
>> distribution kernels (where the randomization seed must be exposed for
>> third party kernel module builds), it still has some value there since
>> now all kernel builds would need to be tracked by an attacker.
>>
>> This introduces two defines __randomize_layout and __no_randomize_layout.
>> Which, in turn, tell the compiler to either randomize or not to randomize
>> the struct in question. Follow-on patches enable the auto-detection
>> logic for selecting structures that contain only function pointers.
>>
>> This feature is ported over from grsecurity.  The implementation is
>> almost entirely identical to the original code written by the PaX Team
>> and Brad Spengler. To make integration simpler, this version has the
>> auto-detection logic temporarily disabled. Structures that are to be
>> randomized are required to use the C99 designated initializer form,
>> which will be in follow-on patches.
>>
>> Signed-off-by: Michael Leibowitz <michael.leibowitz@...el.com>
>> [kees: refreshed plugin, disabled all-fptr-struct detection, update commit log]
>> Signed-off-by: Kees Cook <keescook@...omium.org>
>> ---
>>  Documentation/dontdiff                        |   2 +
>>  arch/Kconfig                                  |  38 ++
>>  include/linux/compiler-gcc.h                  |   5 +
>>  include/linux/compiler.h                      |   8 +
>>  include/linux/vermagic.h                      |   9 +-
>>  kernel/module.c                               |  27 +-
>>  scripts/Makefile.gcc-plugins                  |   5 +
>>  scripts/gcc-plugins/.gitignore                |   1 +
>>  scripts/gcc-plugins/Makefile                  |   8 +
>>  scripts/gcc-plugins/gen-random-seed.sh        |   8 +
>>  scripts/gcc-plugins/randomize_layout_plugin.c | 943 ++++++++++++++++++++++++++
>>  11 files changed, 1052 insertions(+), 2 deletions(-)
>>  create mode 100644 scripts/gcc-plugins/.gitignore
>>  create mode 100644 scripts/gcc-plugins/gen-random-seed.sh
>>  create mode 100644 scripts/gcc-plugins/randomize_layout_plugin.c
>>
>> [...]
>> diff --git a/include/linux/vermagic.h b/include/linux/vermagic.h
>> index 6f8fbcf10dfb..af6c03f7f986 100644
>> --- a/include/linux/vermagic.h
>> +++ b/include/linux/vermagic.h
>> @@ -24,10 +24,17 @@
>>  #ifndef MODULE_ARCH_VERMAGIC
>>  #define MODULE_ARCH_VERMAGIC ""
>>  #endif
>> +#ifdef RANDSTRUCT_PLUGIN
>> +#include <generated/randomize_layout_hash.h>
>> +#define MODULE_RANDSTRUCT_PLUGIN "RANDSTRUCT_PLUGIN_" RANDSTRUCT_HASHED_SEED
>> +#else
>> +#define MODULE_RANDSTRUCT_PLUGIN
>> +#endif
>>
>>  #define VERMAGIC_STRING                                                \
>>         UTS_RELEASE " "                                                 \
>>         MODULE_VERMAGIC_SMP MODULE_VERMAGIC_PREEMPT                     \
>>         MODULE_VERMAGIC_MODULE_UNLOAD MODULE_VERMAGIC_MODVERSIONS       \
>> -       MODULE_ARCH_VERMAGIC
>> +       MODULE_ARCH_VERMAGIC                                            \
>> +       MODULE_RANDSTRUCT_PLUGIN
>>
>> diff --git a/kernel/module.c b/kernel/module.c
>> index f953df992a11..2887660b0e9c 100644
>> --- a/kernel/module.c
>> +++ b/kernel/module.c
>> @@ -1316,13 +1316,30 @@ static int check_version(Elf_Shdr *sechdrs,
>>                 goto bad_version;
>>         }
>>
>> +#ifdef CONFIG_GCC_PLUGIN_RANDSTRUCT
>> +       /*
>> +        * avoid potentially printing jibberish on attempted load
>> +        * of a module randomized with a different seed
>> +        */
>> +       pr_warn("no symbol version for %s\n", symname);
>> +#else
>>         /* Broken toolchain. Warn once, then let it go.. */
>>         pr_warn_once("%s: no symbol version for %s\n", mod->name, symname);
>> +#endif
>>         return 1;
>>
>>  bad_version:
>> +#ifdef CONFIG_GCC_PLUGIN_RANDSTRUCT
>> +       /*
>> +        * avoid potentially printing jibberish on attempted load
>> +        * of a module randomized with a different seed
>> +        */
>> +       pr_warn("attempted module disagrees about version of symbol %s\n",
>> +              symname);
>> +#else
>>         pr_warn("%s: disagrees about version of symbol %s\n",
>>                mod->name, symname);
>> +#endif
>>         return 0;
>>  }
>>
>> @@ -2964,7 +2981,15 @@ static struct module *setup_load_info(struct load_info *info, int flags)
>>         mod = (void *)info->sechdrs[info->index.mod].sh_addr;
>>
>>         if (info->index.sym == 0) {
>> +#ifdef CONFIG_GCC_PLUGIN_RANDSTRUCT
>> +               /*
>> +                * avoid potentially printing jibberish on attempted load
>> +                * of a module randomized with a different seed
>> +                */
>> +               pr_warn("module has no symbols (stripped?)\n");
>> +#else
>>                 pr_warn("%s: module has no symbols (stripped?)\n", mod->name);
>> +#endif
>>                 return ERR_PTR(-ENOEXEC);
>>         }
>>
>
>Hi Jessica,
>
>I was curious to get your input on these module-related changes that
>are needed to support structure layout randomization. In the face of a
>vermagic failure on a kernel built with randstruct, the module errors
>can't dereference the from-disk module structure name field since it
>may not be in the place it's expected.
>
>I was thinking, instead of these explicit #ifdef chunks, maybe making
>a helper macro/function would be better?

Hi Kees,

Yes, that would be preferable to using the repeated #ifdef blocks.
IMO your other patchset which adds name to .modinfo is a cleaner
solution.

(PS, I'm still on PTO, but I'll be able to give that a proper look
after Tues.)

>Also, can we get the module filename from somewhere, and use that
>instead of the name field?

AFAIK, we don't store or keep track of that anywhere in-kernel.

Thanks,

Jessica

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.