[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CA+=Sn1kSg-Y8SseUWPTTJi5HRgYYxVtcDGUJvCcCYQQzKeiUQw@mail.gmail.com>
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
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
