From: Laura Abbott <labbott@redhat.com> To: Kees Cook <keescook@chromium.org>, kernel-hardening@lists.openwall.com Cc: Arnd Bergmann <arnd@arndb.de>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Andrew Morton <akpm@linux-foundation.org>, "David S. Miller" <davem@davemloft.net>, Mauro Carvalho Chehab <mchehab@osg.samsung.com>, Jiri Slaby <jslaby@suse.cz>, Guenter Roeck <linux@roeck-us.net>, linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/5] lkdtm: add function for testing .rodata section Date: Tue, 7 Jun 2016 18:02:01 -0700 [thread overview] Message-ID: <941dc7f5-03a2-8a98-8888-7e92985699e3@redhat.com> (raw) In-Reply-To: <1465336628-18219-4-git-send-email-keescook@chromium.org> On 06/07/2016 02:57 PM, Kees Cook wrote: > This adds a function that lives in the .rodata section. The section > flags are corrected using objcopy since there is no way with gcc to > declare section flags in an architecture-agnostic way. > Permit me to be the bearer of bad architecture news once again. With arm64 cross compiler (both Fedora 6.1.1 and Linaro 5.1) CC drivers/misc/lkdtm_rodata.o OBJCOPY drivers/misc/lkdtm_rodata_objcopy.o LD drivers/misc/lkdtm.o drivers/misc/lkdtm_rodata_objcopy.o: file not recognized: File format not recognized scripts/Makefile.build:423: recipe for target 'drivers/misc/lkdtm.o' failed make[2]: *** [drivers/misc/lkdtm.o] Error 1 scripts/Makefile.build:440: recipe for target 'drivers/misc' failed make[1]: *** [drivers/misc] Error 2 Makefile:985: recipe for target 'drivers' failed make: *** [drivers] Error 2 As far as I can tell this is because arm64 defines OBJCOPYFLAGS and they get propagated to objcopy aarch64-linux-gnu-objcopy -O binary -R .note -R .note.gnu.build-id -R .comment -S --set-section-flags .text=alloc,readonly --rename-section .text=.rodata drivers/misc/lkdtm_rodata.o drivers/misc/lkdtm_rodata_objcopy.o vs x86 objcopy --set-section-flags .text=alloc,readonly --rename-section .text=.rodata drivers/misc/lkdtm_rodata.o drivers/misc/lkdtm_rodata_objcopy.o specifically it's the -O binary that seems to break things, the same failure happens on x86 as well with the the same commands. It works if I clear out the OBJCOPYFLAGS variable first but I don't think that's the correct way to fix this. Thanks, Laura > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > drivers/misc/Makefile | 7 +++++++ > drivers/misc/lkdtm.h | 6 ++++++ > drivers/misc/lkdtm_core.c | 24 +++++++++++++++++------- > drivers/misc/lkdtm_rodata.c | 10 ++++++++++ > 4 files changed, 40 insertions(+), 7 deletions(-) > create mode 100644 drivers/misc/lkdtm.h > create mode 100644 drivers/misc/lkdtm_rodata.c > > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile > index c3cb6ad8cc37..b2d3d68dfa22 100644 > --- a/drivers/misc/Makefile > +++ b/drivers/misc/Makefile > @@ -59,3 +59,10 @@ obj-$(CONFIG_CXL_BASE) += cxl/ > obj-$(CONFIG_PANEL) += panel.o > > lkdtm-$(CONFIG_LKDTM) += lkdtm_core.o > +lkdtm-$(CONFIG_LKDTM) += lkdtm_rodata_objcopy.o > + > +OBJCOPYFLAGS_lkdtm_rodata_objcopy.o := \ > + --set-section-flags .text=alloc,readonly \ > + --rename-section .text=.rodata > +$(obj)/lkdtm_rodata_objcopy.o: $(obj)/lkdtm_rodata.o > + $(call if_changed,objcopy) > diff --git a/drivers/misc/lkdtm.h b/drivers/misc/lkdtm.h > new file mode 100644 > index 000000000000..9531fa3be4c3 > --- /dev/null > +++ b/drivers/misc/lkdtm.h > @@ -0,0 +1,6 @@ > +#ifndef __LKDTM_H > +#define __LKDTM_H > + > +void lkdtm_rodata_do_nothing(void); > + > +#endif > diff --git a/drivers/misc/lkdtm_core.c b/drivers/misc/lkdtm_core.c > index 605050c60f10..187cd9b63e9a 100644 > --- a/drivers/misc/lkdtm_core.c > +++ b/drivers/misc/lkdtm_core.c > @@ -52,6 +52,8 @@ > #include <linux/ide.h> > #endif > > +#include "lkdtm.h" > + > /* > * Make sure our attempts to over run the kernel stack doesn't trigger > * a compiler warning when CONFIG_FRAME_WARN is set. Then make sure we > @@ -103,6 +105,7 @@ enum ctype { > CT_EXEC_STACK, > CT_EXEC_KMALLOC, > CT_EXEC_VMALLOC, > + CT_EXEC_RODATA, > CT_EXEC_USERSPACE, > CT_ACCESS_USERSPACE, > CT_WRITE_RO, > @@ -145,6 +148,7 @@ static char* cp_type[] = { > "EXEC_STACK", > "EXEC_KMALLOC", > "EXEC_VMALLOC", > + "EXEC_RODATA", > "EXEC_USERSPACE", > "ACCESS_USERSPACE", > "WRITE_RO", > @@ -346,15 +350,18 @@ static noinline void corrupt_stack(void) > memset((void *)data, 0, 64); > } > > -static void noinline execute_location(void *dst) > +static noinline void execute_location(void *dst, bool write) > { > void (*func)(void) = dst; > > pr_info("attempting ok execution at %p\n", do_nothing); > do_nothing(); > > - memcpy(dst, do_nothing, EXEC_SIZE); > - flush_icache_range((unsigned long)dst, (unsigned long)dst + EXEC_SIZE); > + if (write) { > + memcpy(dst, do_nothing, EXEC_SIZE); > + flush_icache_range((unsigned long)dst, > + (unsigned long)dst + EXEC_SIZE); > + } > pr_info("attempting bad execution at %p\n", func); > func(); > } > @@ -551,25 +558,28 @@ static void lkdtm_do_action(enum ctype which) > schedule(); > break; > case CT_EXEC_DATA: > - execute_location(data_area); > + execute_location(data_area, true); > break; > case CT_EXEC_STACK: { > u8 stack_area[EXEC_SIZE]; > - execute_location(stack_area); > + execute_location(stack_area, true); > break; > } > case CT_EXEC_KMALLOC: { > u32 *kmalloc_area = kmalloc(EXEC_SIZE, GFP_KERNEL); > - execute_location(kmalloc_area); > + execute_location(kmalloc_area, true); > kfree(kmalloc_area); > break; > } > case CT_EXEC_VMALLOC: { > u32 *vmalloc_area = vmalloc(EXEC_SIZE); > - execute_location(vmalloc_area); > + execute_location(vmalloc_area, true); > vfree(vmalloc_area); > break; > } > + case CT_EXEC_RODATA: > + execute_location(lkdtm_rodata_do_nothing, false); > + break; > case CT_EXEC_USERSPACE: { > unsigned long user_addr; > > diff --git a/drivers/misc/lkdtm_rodata.c b/drivers/misc/lkdtm_rodata.c > new file mode 100644 > index 000000000000..4d0d851f02b9 > --- /dev/null > +++ b/drivers/misc/lkdtm_rodata.c > @@ -0,0 +1,10 @@ > +/* > + * This includes functions that are meant to live entirely in .rodata > + * (via objcopy tricks), to validate the non-executability of .rodata. > + */ > +#include <linux/kernel.h> > + > +void lkdtm_rodata_do_nothing(void) > +{ > + /* Does nothing. We just want an architecture agnostic "return". */ > +} >
WARNING: multiple messages have this Message-ID (diff)
From: Laura Abbott <labbott@redhat.com> To: Kees Cook <keescook@chromium.org>, kernel-hardening@lists.openwall.com Cc: Arnd Bergmann <arnd@arndb.de>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Andrew Morton <akpm@linux-foundation.org>, "David S. Miller" <davem@davemloft.net>, Mauro Carvalho Chehab <mchehab@osg.samsung.com>, Jiri Slaby <jslaby@suse.cz>, Guenter Roeck <linux@roeck-us.net>, linux-kernel@vger.kernel.org Subject: [kernel-hardening] Re: [PATCH 3/5] lkdtm: add function for testing .rodata section Date: Tue, 7 Jun 2016 18:02:01 -0700 [thread overview] Message-ID: <941dc7f5-03a2-8a98-8888-7e92985699e3@redhat.com> (raw) In-Reply-To: <1465336628-18219-4-git-send-email-keescook@chromium.org> On 06/07/2016 02:57 PM, Kees Cook wrote: > This adds a function that lives in the .rodata section. The section > flags are corrected using objcopy since there is no way with gcc to > declare section flags in an architecture-agnostic way. > Permit me to be the bearer of bad architecture news once again. With arm64 cross compiler (both Fedora 6.1.1 and Linaro 5.1) CC drivers/misc/lkdtm_rodata.o OBJCOPY drivers/misc/lkdtm_rodata_objcopy.o LD drivers/misc/lkdtm.o drivers/misc/lkdtm_rodata_objcopy.o: file not recognized: File format not recognized scripts/Makefile.build:423: recipe for target 'drivers/misc/lkdtm.o' failed make[2]: *** [drivers/misc/lkdtm.o] Error 1 scripts/Makefile.build:440: recipe for target 'drivers/misc' failed make[1]: *** [drivers/misc] Error 2 Makefile:985: recipe for target 'drivers' failed make: *** [drivers] Error 2 As far as I can tell this is because arm64 defines OBJCOPYFLAGS and they get propagated to objcopy aarch64-linux-gnu-objcopy -O binary -R .note -R .note.gnu.build-id -R .comment -S --set-section-flags .text=alloc,readonly --rename-section .text=.rodata drivers/misc/lkdtm_rodata.o drivers/misc/lkdtm_rodata_objcopy.o vs x86 objcopy --set-section-flags .text=alloc,readonly --rename-section .text=.rodata drivers/misc/lkdtm_rodata.o drivers/misc/lkdtm_rodata_objcopy.o specifically it's the -O binary that seems to break things, the same failure happens on x86 as well with the the same commands. It works if I clear out the OBJCOPYFLAGS variable first but I don't think that's the correct way to fix this. Thanks, Laura > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > drivers/misc/Makefile | 7 +++++++ > drivers/misc/lkdtm.h | 6 ++++++ > drivers/misc/lkdtm_core.c | 24 +++++++++++++++++------- > drivers/misc/lkdtm_rodata.c | 10 ++++++++++ > 4 files changed, 40 insertions(+), 7 deletions(-) > create mode 100644 drivers/misc/lkdtm.h > create mode 100644 drivers/misc/lkdtm_rodata.c > > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile > index c3cb6ad8cc37..b2d3d68dfa22 100644 > --- a/drivers/misc/Makefile > +++ b/drivers/misc/Makefile > @@ -59,3 +59,10 @@ obj-$(CONFIG_CXL_BASE) += cxl/ > obj-$(CONFIG_PANEL) += panel.o > > lkdtm-$(CONFIG_LKDTM) += lkdtm_core.o > +lkdtm-$(CONFIG_LKDTM) += lkdtm_rodata_objcopy.o > + > +OBJCOPYFLAGS_lkdtm_rodata_objcopy.o := \ > + --set-section-flags .text=alloc,readonly \ > + --rename-section .text=.rodata > +$(obj)/lkdtm_rodata_objcopy.o: $(obj)/lkdtm_rodata.o > + $(call if_changed,objcopy) > diff --git a/drivers/misc/lkdtm.h b/drivers/misc/lkdtm.h > new file mode 100644 > index 000000000000..9531fa3be4c3 > --- /dev/null > +++ b/drivers/misc/lkdtm.h > @@ -0,0 +1,6 @@ > +#ifndef __LKDTM_H > +#define __LKDTM_H > + > +void lkdtm_rodata_do_nothing(void); > + > +#endif > diff --git a/drivers/misc/lkdtm_core.c b/drivers/misc/lkdtm_core.c > index 605050c60f10..187cd9b63e9a 100644 > --- a/drivers/misc/lkdtm_core.c > +++ b/drivers/misc/lkdtm_core.c > @@ -52,6 +52,8 @@ > #include <linux/ide.h> > #endif > > +#include "lkdtm.h" > + > /* > * Make sure our attempts to over run the kernel stack doesn't trigger > * a compiler warning when CONFIG_FRAME_WARN is set. Then make sure we > @@ -103,6 +105,7 @@ enum ctype { > CT_EXEC_STACK, > CT_EXEC_KMALLOC, > CT_EXEC_VMALLOC, > + CT_EXEC_RODATA, > CT_EXEC_USERSPACE, > CT_ACCESS_USERSPACE, > CT_WRITE_RO, > @@ -145,6 +148,7 @@ static char* cp_type[] = { > "EXEC_STACK", > "EXEC_KMALLOC", > "EXEC_VMALLOC", > + "EXEC_RODATA", > "EXEC_USERSPACE", > "ACCESS_USERSPACE", > "WRITE_RO", > @@ -346,15 +350,18 @@ static noinline void corrupt_stack(void) > memset((void *)data, 0, 64); > } > > -static void noinline execute_location(void *dst) > +static noinline void execute_location(void *dst, bool write) > { > void (*func)(void) = dst; > > pr_info("attempting ok execution at %p\n", do_nothing); > do_nothing(); > > - memcpy(dst, do_nothing, EXEC_SIZE); > - flush_icache_range((unsigned long)dst, (unsigned long)dst + EXEC_SIZE); > + if (write) { > + memcpy(dst, do_nothing, EXEC_SIZE); > + flush_icache_range((unsigned long)dst, > + (unsigned long)dst + EXEC_SIZE); > + } > pr_info("attempting bad execution at %p\n", func); > func(); > } > @@ -551,25 +558,28 @@ static void lkdtm_do_action(enum ctype which) > schedule(); > break; > case CT_EXEC_DATA: > - execute_location(data_area); > + execute_location(data_area, true); > break; > case CT_EXEC_STACK: { > u8 stack_area[EXEC_SIZE]; > - execute_location(stack_area); > + execute_location(stack_area, true); > break; > } > case CT_EXEC_KMALLOC: { > u32 *kmalloc_area = kmalloc(EXEC_SIZE, GFP_KERNEL); > - execute_location(kmalloc_area); > + execute_location(kmalloc_area, true); > kfree(kmalloc_area); > break; > } > case CT_EXEC_VMALLOC: { > u32 *vmalloc_area = vmalloc(EXEC_SIZE); > - execute_location(vmalloc_area); > + execute_location(vmalloc_area, true); > vfree(vmalloc_area); > break; > } > + case CT_EXEC_RODATA: > + execute_location(lkdtm_rodata_do_nothing, false); > + break; > case CT_EXEC_USERSPACE: { > unsigned long user_addr; > > diff --git a/drivers/misc/lkdtm_rodata.c b/drivers/misc/lkdtm_rodata.c > new file mode 100644 > index 000000000000..4d0d851f02b9 > --- /dev/null > +++ b/drivers/misc/lkdtm_rodata.c > @@ -0,0 +1,10 @@ > +/* > + * This includes functions that are meant to live entirely in .rodata > + * (via objcopy tricks), to validate the non-executability of .rodata. > + */ > +#include <linux/kernel.h> > + > +void lkdtm_rodata_do_nothing(void) > +{ > + /* Does nothing. We just want an architecture agnostic "return". */ > +} >
next prev parent reply other threads:[~2016-06-08 1:02 UTC|newest] Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-06-07 21:57 [PATCH 0/5] lkdtm: add usercopy and rodata, fix atomic Kees Cook 2016-06-07 21:57 ` [kernel-hardening] " Kees Cook 2016-06-07 21:57 ` [PATCH 1/5] lkdtm: split build into multiple source files Kees Cook 2016-06-07 21:57 ` [kernel-hardening] " Kees Cook 2016-06-07 21:57 ` [PATCH 2/5] lkdtm: clean up after rename Kees Cook 2016-06-07 21:57 ` [kernel-hardening] " Kees Cook 2016-06-07 21:57 ` [PATCH 3/5] lkdtm: add function for testing .rodata section Kees Cook 2016-06-07 21:57 ` [kernel-hardening] " Kees Cook 2016-06-08 1:02 ` Laura Abbott [this message] 2016-06-08 1:02 ` [kernel-hardening] " Laura Abbott 2016-06-08 15:46 ` Kees Cook 2016-06-08 15:46 ` [kernel-hardening] " Kees Cook 2016-06-08 16:15 ` Laura Abbott 2016-06-08 16:15 ` [kernel-hardening] " Laura Abbott 2016-06-08 21:26 ` Kees Cook 2016-06-08 21:26 ` [kernel-hardening] " Kees Cook 2016-06-10 1:14 ` Laura Abbott 2016-06-10 1:14 ` [kernel-hardening] " Laura Abbott 2016-08-01 12:37 ` Linker segfault on powerpc when CONFIG_LKDTM=y (was Re: [kernel-hardening] [PATCH 3/5] lkdtm: add function for testing .rodata section) Michael Ellerman 2016-08-01 12:37 ` [kernel-hardening] " Michael Ellerman 2016-08-01 19:45 ` Kees Cook 2016-08-01 19:45 ` [kernel-hardening] " Kees Cook 2016-08-01 19:45 ` Kees Cook 2016-08-02 3:12 ` [kernel-hardening] " Michael Ellerman 2016-08-02 3:12 ` Michael Ellerman 2016-08-02 3:12 ` Michael Ellerman 2016-08-02 19:12 ` Kees Cook 2016-08-02 19:12 ` Kees Cook 2016-08-03 4:46 ` Michael Ellerman 2016-08-03 4:46 ` Michael Ellerman 2016-08-03 4:46 ` Michael Ellerman 2016-06-07 21:57 ` [PATCH 4/5] lkdtm: add usercopy tests Kees Cook 2016-06-07 21:57 ` [kernel-hardening] " Kees Cook 2016-06-07 23:36 ` kbuild test robot 2016-06-07 23:36 ` [kernel-hardening] " kbuild test robot 2016-06-08 0:07 ` kbuild test robot 2016-06-08 0:07 ` [kernel-hardening] " kbuild test robot 2016-06-07 21:57 ` [PATCH 5/5] lkdtm: split atomic test into over and underflow Kees Cook 2016-06-07 21:57 ` [kernel-hardening] " Kees Cook
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=941dc7f5-03a2-8a98-8888-7e92985699e3@redhat.com \ --to=labbott@redhat.com \ --cc=akpm@linux-foundation.org \ --cc=arnd@arndb.de \ --cc=davem@davemloft.net \ --cc=gregkh@linuxfoundation.org \ --cc=jslaby@suse.cz \ --cc=keescook@chromium.org \ --cc=kernel-hardening@lists.openwall.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux@roeck-us.net \ --cc=mchehab@osg.samsung.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.