Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 30 May 2019 10:09:44 -0700
From: Andrew Pinski <pinskia@...il.com>
To: Tycho Andersen <tycho@...ho.ws>
Cc: GCC Mailing List <gcc@....gnu.org>, kernel-hardening@...ts.openwall.com
Subject: Re: unrecognizable insn generated in plugin?

On Thu, May 30, 2019 at 10:01 AM Tycho Andersen <tycho@...ho.ws> wrote:
>
> Hi all,
>
> I've been trying to implement an idea Andy suggested recently for
> preventing some kinds of ROP attacks. The discussion of the idea is
> here:
> https://lore.kernel.org/linux-mm/DFA69954-3F0F-4B79-A9B5-893D33D87E51@amacapital.net/
>
> Right now I'm struggling to get my plugin to compile without crashing. The
> basic idea is to insert some code before every "pop rbp" and "pop rsp"; I've
> figured out how to find these instructions, and I'm inserting code using:
>
> emit_insn(gen_rtx_XOR(DImode, gen_rtx_REG(DImode, HARD_FRAME_POINTER_REGNUM),
>                       gen_rtx_MEM(DImode, gen_rtx_REG(DImode, HARD_FRAME_POINTER_REGNUM))));

Simplely this xor does not set anything.
I think you want something like:
emit_insn(gen_rtx_SET(gen_rtx_REG(DImode, HARD_FRAME_POINTER_REGNUM),
  gen_rtx_XOR(DImode, gen_rtx_REG(DImode, HARD_FRAME_POINTER_REGNUM),
      gen_rtx_MEM(DImode, gen_rtx_REG(DImode, HARD_FRAME_POINTER_REGNUM)))));

But that might not work either, you might need some thing more.

Thanks,
Andrew Pinski

>
> The plugin completes successfully, but GCC complains later,
>
> kernel/seccomp.c: In function ‘seccomp_check_filter’:
> kernel/seccomp.c:242:1: error: unrecognizable insn:
>  }
>  ^
> (insn 698 645 699 17 (xor:DI (reg:DI 6 bp)
>         (mem:DI (reg:DI 6 bp) [0  S8 A8])) "kernel/seccomp.c":242 -1
>      (nil))
> during RTL pass: shorten
> kernel/seccomp.c:242:1: internal compiler error: in insn_min_length, at config/i386/i386.md:14714
>
> I assume this is because some internal metadata is screwed up, but I have no
> clue as to what that is or how to fix it. My gcc version is 8.3.0, and
> config/i386/i386.md:14714 of that tag looks mostly unrelated.
>
> I had problems earlier because I was trying to run it after *clean_state which
> is the thing that does init_insn_lengths(), but now I'm running it after
> *stack_regs, so I thought it should be ok...
>
> Anyway, the full plugin draft is below. You can run it by adding
> CONFIG_GCC_PLUGIN_HEAPLEAP=y to your kernel config.
>
> Thanks!
>
> Tycho
>
>
> From 83b0631f14784ce11362ebd64e40c8d25c0decee Mon Sep 17 00:00:00 2001
> From: Tycho Andersen <tycho@...ho.ws>
> Date: Fri, 19 Apr 2019 19:24:58 -0600
> Subject: [PATCH] heapleap
>
> Signed-off-by: Tycho Andersen <tycho@...ho.ws>
> ---
>  scripts/Makefile.gcc-plugins          |   8 ++
>  scripts/gcc-plugins/Kconfig           |   4 +
>  scripts/gcc-plugins/heapleap_plugin.c | 189 ++++++++++++++++++++++++++
>  3 files changed, 201 insertions(+)
>
> diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins
> index 5f7df50cfe7a..283b81dc5742 100644
> --- a/scripts/Makefile.gcc-plugins
> +++ b/scripts/Makefile.gcc-plugins
> @@ -44,6 +44,14 @@ ifdef CONFIG_GCC_PLUGIN_ARM_SSP_PER_TASK
>  endif
>  export DISABLE_ARM_SSP_PER_TASK_PLUGIN
>
> +gcc-plugin-$(CONFIG_GCC_PLUGIN_HEAPLEAP)       += heapleap_plugin.so
> +gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_HEAPLEAP)                \
> +                       += -DHEAPLEAP_PLUGIN
> +ifdef CONFIG_GCC_PLUGIN_HEAPLEAP
> +    DISABLE_HEAPLEAP_PLUGIN += -fplugin-arg-heapleap_plugin-disable
> +endif
> +export DISABLE_HEAPLEAP_PLUGIN
> +
>  # All the plugin CFLAGS are collected here in case a build target needs to
>  # filter them out of the KBUILD_CFLAGS.
>  GCC_PLUGINS_CFLAGS := $(strip $(addprefix -fplugin=$(objtree)/scripts/gcc-plugins/, $(gcc-plugin-y)) $(gcc-plugin-cflags-y))
> diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
> index 74271dba4f94..491b9cd5df1a 100644
> --- a/scripts/gcc-plugins/Kconfig
> +++ b/scripts/gcc-plugins/Kconfig
> @@ -226,4 +226,8 @@ config GCC_PLUGIN_ARM_SSP_PER_TASK
>         bool
>         depends on GCC_PLUGINS && ARM
>
> +config GCC_PLUGIN_HEAPLEAP
> +       bool "Prevent 'pop esp' type instructions from loading an address in the heap"
> +       depends on GCC_PLUGINS
> +
>  endif
> diff --git a/scripts/gcc-plugins/heapleap_plugin.c b/scripts/gcc-plugins/heapleap_plugin.c
> new file mode 100644
> index 000000000000..5051b96d79f4
> --- /dev/null
> +++ b/scripts/gcc-plugins/heapleap_plugin.c
> @@ -0,0 +1,189 @@
> +/*
> + * This is based on an idea from Andy Lutomirski described here:
> + * https://lore.kernel.org/linux-mm/DFA69954-3F0F-4B79-A9B5-893D33D87E51@amacapital.net/
> + *
> + * unsigned long offset = *rsp - rsp;
> + * offset >>= THREAD_SHIFT;
> + * if (unlikely(offset))
> + *     BUG();
> + * POP RSP;
> + */
> +
> +#include "gcc-common.h"
> +
> +__visible int plugin_is_GPL_compatible;
> +static bool disable = false;
> +
> +static struct plugin_info heapleap_plugin_info = {
> +       .version = "1",
> +       .help = "disable\t\tdo not activate the plugin\n"
> +};
> +
> +static bool heapleap_gate(void)
> +{
> +       tree section;
> +
> +       /*
> +        * Similar to stackleak, we only do this for user code for now.
> +        */
> +       section = lookup_attribute("section",
> +                                  DECL_ATTRIBUTES(current_function_decl));
> +       if (section && TREE_VALUE(section)) {
> +               section = TREE_VALUE(TREE_VALUE(section));
> +
> +               if (!strncmp(TREE_STRING_POINTER(section), ".init.text", 10))
> +                       return false;
> +               if (!strncmp(TREE_STRING_POINTER(section), ".devinit.text", 13))
> +                       return false;
> +               if (!strncmp(TREE_STRING_POINTER(section), ".cpuinit.text", 13))
> +                       return false;
> +               if (!strncmp(TREE_STRING_POINTER(section), ".meminit.text", 13))
> +                       return false;
> +       }
> +
> +       return !disable;
> +}
> +
> +/*
> + * Check that:
> + *
> + * unsigned long offset = *rbp - rbp;
> + * offset >>= THREAD_SHIFT;
> + * if (unlikely(offset))
> + *     BUG();
> + * pop rbp;
> + *
> + * (we should probably do the same for rsp?)
> + */
> +static void heapleap_add_check(rtx_insn *insn)
> +{
> +       rtx_insn *seq_head;
> +
> +       fprintf(stderr, "adding heapleap check\n");
> +       print_rtl_single(stderr, insn);
> +
> +       start_sequence();
> +
> +       /* xor ebp [ebp] */
> +       emit_insn(gen_rtx_XOR(DImode, gen_rtx_REG(DImode, HARD_FRAME_POINTER_REGNUM),
> +                             gen_rtx_MEM(DImode, gen_rtx_REG(DImode, HARD_FRAME_POINTER_REGNUM))));
> +
> +       /* ebp >> THREAD_SHIFT */
> +       /*
> +        * TODO: THREAD_SHIFT isn't defined for every arch, including x86.
> +        * THREAD_SIZE for x86_64 is 4096 * 2, so THREAD_SHIFT would be 13
> +        * there. We should at least compute this from THREAD_SIZE though.
> +        */
> +       emit_insn(gen_rtx_LSHIFTRT(DImode, gen_rtx_REG(DImode, HARD_FRAME_POINTER_REGNUM),
> +                                  GEN_INT(13)));
> +
> +       /*
> +        * We're inserting right before the final pass, and we're adding some
> +        * kind of jump, thus splitting the basic block that is the epilogue.
> +        * That probably causes problems, and currently gcc crashes when doing
> +        * the final pass after we emit this, so we probably need to do better.
> +        */
> +       emit_insn(gen_rtx_IF_THEN_ELSE(DImode,
> +                       gen_rtx_NE(DImode,
> +                               gen_rtx_REG(DImode, HARD_FRAME_POINTER_REGNUM),
> +                               GEN_INT(0)),
> +                       /*
> +                        * we're really not supposed to BUG() for this stuff;
> +                        * maybe we should figure out how to call WARN()? might
> +                        * be painful.
> +                        */
> +                       gen_ud2(),
> +                       /* poor man's no-op, i.e. how do i do this better? */
> +                       gen_rtx_SET(gen_rtx_REG(DImode, HARD_FRAME_POINTER_REGNUM),
> +                                   gen_rtx_REG(DImode, HARD_FRAME_POINTER_REGNUM))));
> +       seq_head = get_insns();
> +       end_sequence();
> +
> +       emit_insn_before(seq_head, insn);
> +}
> +
> +static unsigned int heapleap_execute(void)
> +{
> +       rtx_insn *insn, *next;
> +
> +       if (strcmp(IDENTIFIER_POINTER(DECL_NAME(cfun->decl)), "seccomp_check_filter"))
> +               return 0;
> +
> +       for (insn = get_insns(); insn; insn = next) {
> +               rtx body, set, lhs, rhs;
> +               int i;
> +
> +               next = NEXT_INSN(insn);
> +               if (!next)
> +                       continue;
> +
> +               if (!RTX_FRAME_RELATED_P(next) || !NONJUMP_INSN_P(next))
> +                       continue;
> +
> +               /*
> +                * I don't understand why we need this; but PATTERN(insn) is a
> +                * CODE_LABEL, so...
> +                */
> +               body = XEXP(insn, 1);
> +               set = PATTERN(body);
> +               if (GET_CODE(set) != SET)
> +                       continue;
> +
> +               /* TODO: use SET_DEST() here instead? */
> +               lhs = XEXP(set, 0);
> +               /* TODO: ebp vs esp? esp only occurs twice in my linked kernel */
> +               if (GET_CODE(lhs) != REG || REGNO(lhs) != HARD_FRAME_POINTER_REGNUM)
> +                       continue;
> +
> +               /* TODO: use SET_SRC() here instead? */
> +               rhs = XEXP(set, 1);
> +               if (GET_CODE(rhs) != MEM)
> +                       continue;
> +
> +               heapleap_add_check(next);
> +       }
> +
> +       return 0;
> +}
> +
> +#define PASS_NAME heapleap
> +#include "gcc-generate-rtl-pass.h"
> +
> +__visible int plugin_init(struct plugin_name_args *plugin_info,
> +                         struct plugin_gcc_version *version)
> +{
> +       const char * const plugin_name = plugin_info->base_name;
> +       const int argc = plugin_info->argc;
> +       const struct plugin_argument * const argv = plugin_info->argv;
> +       int i;
> +
> +       /*
> +        * *clean_state is the pass that does init_insn_lengths(), so we can't
> +        * do anything after this, because gcc fails there's not a length for
> +        * every instruction in the final pass
> +        */
> +       PASS_INFO(heapleap, "*stack_regs", 1, PASS_POS_INSERT_AFTER);
> +
> +       if (!plugin_default_version_check(version, &gcc_version)) {
> +               error(G_("incompatible gcc/plugin versions"));
> +               return 1;
> +       }
> +
> +       for (i = 0; i < argc; i++) {
> +               if (!strcmp(argv[i].key, "disable")) {
> +                       disable = true;
> +                       return 0;
> +               } else {
> +                       error(G_("unknown option '-fplugin-arg-%s-%s'"),
> +                                       plugin_name, argv[i].key);
> +                       return 1;
> +               }
> +       }
> +
> +       register_callback(plugin_name, PLUGIN_INFO, NULL,
> +                                               &heapleap_plugin_info);
> +
> +       register_callback(plugin_name, PLUGIN_PASS_MANAGER_SETUP, NULL,
> +                                       &heapleap_pass_info);
> +       return 0;
> +}
> --
> 2.20.1
>

Powered by blists - more mailing lists

Your e-mail address:

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