| Message ID | 1465336628-18219-4-git-send-email-keescook@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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". */ > +} >
On Tue, Jun 7, 2016 at 6:02 PM, Laura Abbott <labbott@redhat.com> wrote: > 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) Uhhhh... How is arm64 injecting those extra flags? OBJCOPYFLAGS is being set with := here? In related news I need to figure out how to get my cross-compiler builds more well scripted... -Kees
On 06/08/2016 08:46 AM, Kees Cook wrote: > On Tue, Jun 7, 2016 at 6:02 PM, Laura Abbott <labbott@redhat.com> wrote: >> 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) > > Uhhhh... How is arm64 injecting those extra flags? OBJCOPYFLAGS is > being set with := here? > Looks like intended behavior from scripts/Makefile.lib: # Objcopy # --------------------------------------------------------------------------- quiet_cmd_objcopy = OBJCOPY $@ cmd_objcopy = $(OBJCOPY) $(OBJCOPYFLAGS) $(OBJCOPYFLAGS_$(@F)) $< $@ OBJCOPYFLAGS_$@ is separate from OBJCOPYFLAGS which makes sense for the original intentions although not what we want here. Some Makefile wizardry is probably needed. > In related news I need to figure out how to get my cross-compiler > builds more well scripted... > > -Kees >
On Wed, Jun 8, 2016 at 8:46 AM, Kees Cook <keescook@chromium.org> wrote: > On Tue, Jun 7, 2016 at 6:02 PM, Laura Abbott <labbott@redhat.com> wrote: >> 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 >>> + If I add: OBJCOPYFLAGS := here, it seems to fix things... >>> +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) > > Uhhhh... How is arm64 injecting those extra flags? OBJCOPYFLAGS is > being set with := here? > > In related news I need to figure out how to get my cross-compiler > builds more well scripted... > > -Kees > > -- > Kees Cook > Chrome OS & Brillo Security
On 06/08/2016 02:26 PM, Kees Cook wrote: > On Wed, Jun 8, 2016 at 8:46 AM, Kees Cook <keescook@chromium.org> wrote: >> On Tue, Jun 7, 2016 at 6:02 PM, Laura Abbott <labbott@redhat.com> wrote: >>> 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 >>>> + > > If I add: > > OBJCOPYFLAGS := > > here, it seems to fix things... > I can confirm that this works on arm64 # echo EXEC_RODATA > /sys/kernel/debug/provoke-crash/DIRECT [ 21.725514] lkdtm: Performing direct entry EXEC_RODATA [ 21.725890] lkdtm: attempting ok execution at ffff0000084c0e08 [ 21.726030] lkdtm: attempting bad execution at ffff000008880700 [ 21.726401] Bad mode in Synchronous Abort handler detected on CPU2, code 0x8400000e -- IABT (current EL) [ 21.726847] CPU: 2 PID: 998 Comm: sh Not tainted 4.7.0-rc2+ #13 I wish the exception was clearer what the actual error was. I might propose a patch to make it more obvious. Thanks, Laura
Kees Cook <keescook@chromium.org> writes: > 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. > > 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 This is blowing up my linker :( scripts/link-vmlinux.sh: line 52: 36260 Segmentation fault (core dumped) ${LD} ${LDFLAGS} ${LDFLAGS_vmlinux} -o ${2} -T ${lds} ${KBUILD_VMLINUX_INIT} --start-group ${KBUILD_VMLINUX_MAIN} --end-group ${1} Haven't had a chance to debug it further. cheers
On Mon, Aug 1, 2016 at 5:37 AM, Michael Ellerman <mpe@ellerman.id.au> wrote: > Kees Cook <keescook@chromium.org> writes: > >> 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. >> >> 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 > > This is blowing up my linker :( > > scripts/link-vmlinux.sh: line 52: 36260 Segmentation fault (core dumped) ${LD} ${LDFLAGS} ${LDFLAGS_vmlinux} -o ${2} -T ${lds} ${KBUILD_VMLINUX_INIT} --start-group ${KBUILD_VMLINUX_MAIN} --end-group ${1} > > Haven't had a chance to debug it further. Argh. Do you want a quick fix for this now? I can add a PPC CONFIG blacklist for the rodata check, maybe? Also, what version of gcc? I'll see if I can reproduce this with a cross compiler... -Kees
Kees Cook <keescook@chromium.org> writes: > On Mon, Aug 1, 2016 at 5:37 AM, Michael Ellerman <mpe@ellerman.id.au> wrote: >> Kees Cook <keescook@chromium.org> writes: >> >>> 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. >>> >>> 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 >> >> This is blowing up my linker :( >> >> scripts/link-vmlinux.sh: line 52: 36260 Segmentation fault (core dumped) ${LD} ${LDFLAGS} ${LDFLAGS_vmlinux} -o ${2} -T ${lds} ${KBUILD_VMLINUX_INIT} --start-group ${KBUILD_VMLINUX_MAIN} --end-group ${1} >> >> Haven't had a chance to debug it further. > > Argh. Do you want a quick fix for this now? I can add a PPC CONFIG > blacklist for the rodata check, maybe? Nah that's OK, none of our defconfigs have it enabled so it's not a real blocker. It also builds OK as a module - though I haven't tested the result yet. > Also, what version of gcc? I'll see if I can reproduce this with a > cross compiler... The original hit was with gcc-5.3 (which is actually a x86->ppc cross): http://kisskb.ellerman.id.au/kisskb/buildresult/12762730/ But I can also reproduce with 5.4, and 6.1.0. Interestingly I *can't* reproduce with the Ubuntu x86->ppc cross (5.4.0-6ubuntu1~16.04.1). Those toolchains are all using binutils 2.26 AFAIK. Going back to a really old toolchain (gcc 4.6.3/binutils 2.22) it does build but I get these warnings: powerpc64-linux-ld: drivers/misc/built-in.o: .opd is not a regular array of opd entries powerpc64-linux-ld: drivers/built-in.o: .opd is not a regular array of opd entries powerpc64-linux-ld: drivers/built-in.o: .opd is not a regular array of opd entries powerpc64-linux-ld: drivers/built-in.o: .opd is not a regular array of opd entries powerpc64-linux-ld: drivers/built-in.o: .opd is not a regular array of opd entries So probably don't worry about it and we'll try and work it out on our end. cheers
On Mon, Aug 1, 2016 at 8:12 PM, Michael Ellerman <mpe@ellerman.id.au> wrote: > Kees Cook <keescook@chromium.org> writes: > >> On Mon, Aug 1, 2016 at 5:37 AM, Michael Ellerman <mpe@ellerman.id.au> wrote: >>> Kees Cook <keescook@chromium.org> writes: >>> >>>> 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. >>>> >>>> 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 >>> >>> This is blowing up my linker :( >>> >>> scripts/link-vmlinux.sh: line 52: 36260 Segmentation fault (core dumped) ${LD} ${LDFLAGS} ${LDFLAGS_vmlinux} -o ${2} -T ${lds} ${KBUILD_VMLINUX_INIT} --start-group ${KBUILD_VMLINUX_MAIN} --end-group ${1} >>> >>> Haven't had a chance to debug it further. >> >> Argh. Do you want a quick fix for this now? I can add a PPC CONFIG >> blacklist for the rodata check, maybe? > > Nah that's OK, none of our defconfigs have it enabled so it's not a real > blocker. It also builds OK as a module - though I haven't tested the > result yet. Okay, I'll leave it as is. >> Also, what version of gcc? I'll see if I can reproduce this with a >> cross compiler... > > The original hit was with gcc-5.3 (which is actually a x86->ppc cross): > > http://kisskb.ellerman.id.au/kisskb/buildresult/12762730/ > > But I can also reproduce with 5.4, and 6.1.0. > > Interestingly I *can't* reproduce with the Ubuntu x86->ppc cross > (5.4.0-6ubuntu1~16.04.1). Oh, weird. Well, that does explains my lack of hitting the problem, though: that's the cross compiler I was using. :P > Those toolchains are all using binutils 2.26 AFAIK. I wonder if this is some gold vs bfd issue, or a specific bug that got fixed in the Ubuntu tree but hasn't landed in 6.1 or 5.4 (??) > Going back to a really old toolchain (gcc 4.6.3/binutils 2.22) it does > build but I get these warnings: > > powerpc64-linux-ld: drivers/misc/built-in.o: .opd is not a regular array of opd entries > powerpc64-linux-ld: drivers/built-in.o: .opd is not a regular array of opd entries How strange. I wonder if there's some corner case of the objcopy that is wrong... > So probably don't worry about it and we'll try and work it out on our end. Okay, sounds good. -Kees
Kees Cook <keescook@chromium.org> writes: > On Mon, Aug 1, 2016 at 8:12 PM, Michael Ellerman <mpe@ellerman.id.au> wrote: >> Kees Cook <keescook@chromium.org> writes: >>> On Mon, Aug 1, 2016 at 5:37 AM, Michael Ellerman <mpe@ellerman.id.au> wrote: >>>> >>>> scripts/link-vmlinux.sh: line 52: 36260 Segmentation fault (core dumped) ${LD} ${LDFLAGS} ${LDFLAGS_vmlinux} -o ${2} -T ${lds} ${KBUILD_VMLINUX_INIT} --start-group ${KBUILD_VMLINUX_MAIN} --end-group ${1} >>>> >>>> Haven't had a chance to debug it further. ... >> Interestingly I *can't* reproduce with the Ubuntu x86->ppc cross >> (5.4.0-6ubuntu1~16.04.1). > > Oh, weird. Well, that does explains my lack of hitting the problem, > though: that's the cross compiler I was using. :P Actually that was a false negative. The trick is you have to have LKDTM=y *and* FUNCTION_TRACER=y. It is a linker bug: https://sourceware.org/bugzilla/show_bug.cgi?id=20428 Which Alan has already fixed. But we need to workaround existing linkers that are out there. We can do that by marking lkdtm_rodata_do_nothing() notrace, which I think makes sense for all arches actually. So I'll send you a patch to do that. cheers
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". */ +}
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. 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