unrecognizable insn generated in plugin?
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-893d33d87...@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;
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
Date: Fri, 19 Apr 2019 19:24:58 -0600
Subject: [PATCH] heapleap
Signed-off-by: Tycho Andersen
---
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 ..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-893d33d87...@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 (!strncm
Re: unrecognizable insn generated in plugin?
Hi Andrew, On Thu, May 30, 2019 at 10:09:44AM -0700, Andrew Pinski wrote: > On Thu, May 30, 2019 at 10:01 AM Tycho Andersen 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-893d33d87...@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. Yes, thanks. You're also right that I still see the same problem: kernel/seccomp.c: In function ‘seccomp_check_filter’: kernel/seccomp.c:242:1: error: unrecognizable insn: } ^ (insn 698 645 699 17 (set (reg:DI 6 bp) (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 Thanks, Tycho
Re: unrecognizable insn generated in plugin?
On Fri, May 31, 2019 at 05:43:44PM +0200, Mark Brand wrote: > On Thu, May 30, 2019 at 9:26 PM Tycho Andersen wrote: > > > > Hi Andrew, > > > > On Thu, May 30, 2019 at 10:09:44AM -0700, Andrew Pinski wrote: > > > On Thu, May 30, 2019 at 10:01 AM Tycho Andersen 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-893d33d87...@amacapital.net/ > > > > > > > Hi Tycho, > > I realise this is maybe not relevant to the topic of fixing the > plugin; but I'm struggling to understand what this is intending to > protect against. > > The idea seems to be to make sure that restored rbp, rsp values are > "close" to the current rbp, rsp values? The only scenario I can see > this providing any benefit is if an attacker can only corrupt a saved > stack/frame pointer, which seems like such an unlikely situation that > it's not really worth adding any complexity to defend against. > An attacker who has control of rip can surely get a controlled value > into rsp in various ways; Yes, if you already have control of rip this doesn't help you. > a quick scan of the current Ubuntu 18.04 > kernel image offers the following sequence (which shows up > everywhere): > > lea rsp, qword ptr [r10 - 8] > ret > > I'd assume that it's not tremendously difficult for an attacker to > chain to this without needing to previously pivot out the stack > pointer, assuming that at the point at which they gain control of rip > they have control over some state somewhere. If you could explain the > exact attack scenario that you have in mind then perhaps I could > provide a better explanation of how one might bypass it. The core bit that's important here is the writes to rsp/rbp, not the fact that they're pop instructions. The insight is that we know how the thread's stack should be aligned, and so any value that's written to these registers outside of that alignment (during "normal" execution) is a bug. The idea is that a ROP attack requires a payload to be injected somewhere so that it can return to this payload and execute this attack. This is most probably done via some allocation elsewhere (add_key() or unreceived data or whatever) since as you note, the stack should be mostly well protected. So then, if we don't allow anyone to write anything that's not on the stack to rsp/rbp, in principle we should be safe from ROP attacks where the payload is elsewhere. As you note, preventing bad "pop rpb" instructions is not enough, nor is preventing bad "pop rsp", as Andy initially proposed. We need to prevent all bad writes to these registers, including the sequence you mentioned, and presumably others. So there would need to be a lot more matching and checks inserted, and maybe it would ultimately be slow. Right now I just wanted to play with it for a bit to see if I can get it to work at all even in one case :) Am I thinking about this wrong? Any discussion is welcome, thanks! Tycho
