* [PATCH] lkdtm: support llvm-objcopy @ 2019-05-13 22:21 Nick Desaulniers 2019-05-13 22:26 ` Nick Desaulniers ` (2 more replies) 0 siblings, 3 replies; 16+ messages in thread From: Nick Desaulniers @ 2019-05-13 22:21 UTC (permalink / raw) To: keescook Cc: clang-built-linux, Nick Desaulniers, Nathan Chancellor, Jordan Rupprect, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel With CONFIG_LKDTM=y and make OBJCOPY=llvm-objcopy, llvm-objcopy errors: llvm-objcopy: error: --set-section-flags=.text conflicts with --rename-section=.text=.rodata Rather than support setting flags then renaming sections vs renaming then setting flags, it's simpler to just change both at the same time via --rename-section. This can be verified with: $ readelf -S drivers/misc/lkdtm/rodata_objcopy.o ... Section Headers: [Nr] Name Type Address Offset Size EntSize Flags Link Info Align ... [ 1] .rodata PROGBITS 0000000000000000 00000040 0000000000000004 0000000000000000 A 0 0 4 ... Which shows in the Flags field that .text is now renamed .rodata, the append flag A is set, and the section is not flagged as writeable W. Link: https://github.com/ClangBuiltLinux/linux/issues/448 Reported-by: Nathan Chancellor <nathanchance@gmail.com> Suggested-by: Jordan Rupprect <rupprecht@google.com> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> --- drivers/misc/lkdtm/Makefile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/misc/lkdtm/Makefile b/drivers/misc/lkdtm/Makefile index 951c984de61a..89dee2a9d88c 100644 --- a/drivers/misc/lkdtm/Makefile +++ b/drivers/misc/lkdtm/Makefile @@ -15,8 +15,7 @@ KCOV_INSTRUMENT_rodata.o := n OBJCOPYFLAGS := OBJCOPYFLAGS_rodata_objcopy.o := \ - --set-section-flags .text=alloc,readonly \ - --rename-section .text=.rodata + --rename-section .text=.rodata,alloc,readonly targets += rodata.o rodata_objcopy.o $(obj)/rodata_objcopy.o: $(obj)/rodata.o FORCE $(call if_changed,objcopy) -- 2.21.0.1020.gf2820cf01a-goog ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] lkdtm: support llvm-objcopy 2019-05-13 22:21 [PATCH] lkdtm: support llvm-objcopy Nick Desaulniers @ 2019-05-13 22:26 ` Nick Desaulniers 2019-05-13 23:04 ` Kees Cook 2019-05-13 23:29 ` Nathan Chancellor 2 siblings, 0 replies; 16+ messages in thread From: Nick Desaulniers @ 2019-05-13 22:26 UTC (permalink / raw) To: Kees Cook Cc: clang-built-linux, Nathan Chancellor, Jordan Rupprect, Arnd Bergmann, Greg Kroah-Hartman, LKML On Mon, May 13, 2019 at 3:21 PM Nick Desaulniers <ndesaulniers@google.com> wrote: > > With CONFIG_LKDTM=y and make OBJCOPY=llvm-objcopy, llvm-objcopy errors: > llvm-objcopy: error: --set-section-flags=.text conflicts with > --rename-section=.text=.rodata > > Rather than support setting flags then renaming sections vs renaming > then setting flags, it's simpler to just change both at the same time > via --rename-section. > > This can be verified with: > $ readelf -S drivers/misc/lkdtm/rodata_objcopy.o > ... > Section Headers: > [Nr] Name Type Address Offset > Size EntSize Flags Link Info Align > ... > [ 1] .rodata PROGBITS 0000000000000000 00000040 > 0000000000000004 0000000000000000 A 0 0 4 > ... > > Which shows in the Flags field that .text is now renamed .rodata, the > append flag A is set, and the section is not flagged as writeable W. Probably should've been: Which shows that .text is now renamed .rodata, the append flag A is set, and the section is not flagged as writeable W. -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] lkdtm: support llvm-objcopy 2019-05-13 22:21 [PATCH] lkdtm: support llvm-objcopy Nick Desaulniers 2019-05-13 22:26 ` Nick Desaulniers @ 2019-05-13 23:04 ` Kees Cook 2019-05-13 23:29 ` Nathan Chancellor 2 siblings, 0 replies; 16+ messages in thread From: Kees Cook @ 2019-05-13 23:04 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: clang-built-linux, Nick Desaulniers, Nathan Chancellor, Jordan Rupprect, Arnd Bergmann, linux-kernel On Mon, May 13, 2019 at 03:21:09PM -0700, Nick Desaulniers wrote: > With CONFIG_LKDTM=y and make OBJCOPY=llvm-objcopy, llvm-objcopy errors: > llvm-objcopy: error: --set-section-flags=.text conflicts with > --rename-section=.text=.rodata > > Rather than support setting flags then renaming sections vs renaming > then setting flags, it's simpler to just change both at the same time > via --rename-section. > > This can be verified with: > $ readelf -S drivers/misc/lkdtm/rodata_objcopy.o > ... > Section Headers: > [Nr] Name Type Address Offset > Size EntSize Flags Link Info Align > ... > [ 1] .rodata PROGBITS 0000000000000000 00000040 > 0000000000000004 0000000000000000 A 0 0 4 > ... > > Which shows in the Flags field that .text is now renamed .rodata, the > append flag A is set, and the section is not flagged as writeable W. > > Link: https://github.com/ClangBuiltLinux/linux/issues/448 > Reported-by: Nathan Chancellor <nathanchance@gmail.com> > Suggested-by: Jordan Rupprect <rupprecht@google.com> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> Thanks! This looks good. Greg, can you please take this for drivers/misc? Acked-by: Kees Cook <keescook@chromium.org> -Kees > --- > drivers/misc/lkdtm/Makefile | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/misc/lkdtm/Makefile b/drivers/misc/lkdtm/Makefile > index 951c984de61a..89dee2a9d88c 100644 > --- a/drivers/misc/lkdtm/Makefile > +++ b/drivers/misc/lkdtm/Makefile > @@ -15,8 +15,7 @@ KCOV_INSTRUMENT_rodata.o := n > > OBJCOPYFLAGS := > OBJCOPYFLAGS_rodata_objcopy.o := \ > - --set-section-flags .text=alloc,readonly \ > - --rename-section .text=.rodata > + --rename-section .text=.rodata,alloc,readonly > targets += rodata.o rodata_objcopy.o > $(obj)/rodata_objcopy.o: $(obj)/rodata.o FORCE > $(call if_changed,objcopy) > -- > 2.21.0.1020.gf2820cf01a-goog > -- Kees Cook ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] lkdtm: support llvm-objcopy 2019-05-13 22:21 [PATCH] lkdtm: support llvm-objcopy Nick Desaulniers 2019-05-13 22:26 ` Nick Desaulniers 2019-05-13 23:04 ` Kees Cook @ 2019-05-13 23:29 ` Nathan Chancellor 2019-05-13 23:38 ` Jordan Rupprecht 2019-05-13 23:50 ` Nick Desaulniers 2 siblings, 2 replies; 16+ messages in thread From: Nathan Chancellor @ 2019-05-13 23:29 UTC (permalink / raw) To: Nick Desaulniers Cc: keescook, clang-built-linux, Nathan Chancellor, Jordan Rupprect, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel On Mon, May 13, 2019 at 03:21:09PM -0700, 'Nick Desaulniers' via Clang Built Linux wrote: > With CONFIG_LKDTM=y and make OBJCOPY=llvm-objcopy, llvm-objcopy errors: > llvm-objcopy: error: --set-section-flags=.text conflicts with > --rename-section=.text=.rodata > > Rather than support setting flags then renaming sections vs renaming > then setting flags, it's simpler to just change both at the same time > via --rename-section. > > This can be verified with: > $ readelf -S drivers/misc/lkdtm/rodata_objcopy.o > ... > Section Headers: > [Nr] Name Type Address Offset > Size EntSize Flags Link Info Align > ... > [ 1] .rodata PROGBITS 0000000000000000 00000040 > 0000000000000004 0000000000000000 A 0 0 4 > ... > > Which shows in the Flags field that .text is now renamed .rodata, the > append flag A is set, and the section is not flagged as writeable W. > > Link: https://github.com/ClangBuiltLinux/linux/issues/448 > Reported-by: Nathan Chancellor <nathanchance@gmail.com> This should be natechancellor@gmail.com (although I think I do own that email, just haven't been into it for 10+ years...) > Suggested-by: Jordan Rupprect <rupprecht@google.com> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > --- > drivers/misc/lkdtm/Makefile | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/misc/lkdtm/Makefile b/drivers/misc/lkdtm/Makefile > index 951c984de61a..89dee2a9d88c 100644 > --- a/drivers/misc/lkdtm/Makefile > +++ b/drivers/misc/lkdtm/Makefile > @@ -15,8 +15,7 @@ KCOV_INSTRUMENT_rodata.o := n > > OBJCOPYFLAGS := > OBJCOPYFLAGS_rodata_objcopy.o := \ > - --set-section-flags .text=alloc,readonly \ > - --rename-section .text=.rodata > + --rename-section .text=.rodata,alloc,readonly > targets += rodata.o rodata_objcopy.o > $(obj)/rodata_objcopy.o: $(obj)/rodata.o FORCE > $(call if_changed,objcopy) > -- > 2.21.0.1020.gf2820cf01a-goog > I ran this script to see if there was any change for GNU objcopy and it looks like .rodata's type gets changed, is this intentional? Otherwise, this works for llvm-objcopy like you show. ----------- 1c1 < There are 11 section headers, starting at offset 0x240: --- > There are 11 section headers, starting at offset 0x230: 8c8 < [ 1] .rodata PROGBITS 0000000000000000 00000040 --- > [ 1] .rodata NOBITS 0000000000000000 00000040 10c10 ----------- #!/bin/bash TMP1=$(mktemp) TMP2=$(mktemp) git checkout next-20190513 make -j$(nproc) ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- O=out mrproper allyesconfig drivers/misc/lkdtm/ readelf -S out/drivers/misc/lkdtm/rodata_objcopy.o > ${TMP1} curl -LSs https://lore.kernel.org/lkml/20190513222109.110020-1-ndesaulniers@google.com/raw | git am -3 make -j$(nproc) ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- O=out mrproper allyesconfig drivers/misc/lkdtm/ readelf -S out/drivers/misc/lkdtm/rodata_objcopy.o > ${TMP2} diff ${TMP1} ${TMP2} ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] lkdtm: support llvm-objcopy 2019-05-13 23:29 ` Nathan Chancellor @ 2019-05-13 23:38 ` Jordan Rupprecht 2019-05-13 23:47 ` Nathan Chancellor 2019-05-13 23:50 ` Nick Desaulniers 1 sibling, 1 reply; 16+ messages in thread From: Jordan Rupprecht @ 2019-05-13 23:38 UTC (permalink / raw) To: Nathan Chancellor Cc: Nick Desaulniers, keescook, clang-built-linux, Nathan Chancellor, Arnd Bergmann, Greg Kroah-Hartman, LKML [-- Attachment #1: Type: text/plain, Size: 3633 bytes --] Nathan: is your version of llvm-objcopy later than r359639 (April 30)? From: Nathan Chancellor <natechancellor@gmail.com> Date: Mon, May 13, 2019 at 4:29 PM To: Nick Desaulniers Cc: <keescook@chromium.org>, <clang-built-linux@googlegroups.com>, Nathan Chancellor, Jordan Rupprect, Arnd Bergmann, Greg Kroah-Hartman, <linux-kernel@vger.kernel.org> > On Mon, May 13, 2019 at 03:21:09PM -0700, 'Nick Desaulniers' via Clang Built Linux wrote: > > With CONFIG_LKDTM=y and make OBJCOPY=llvm-objcopy, llvm-objcopy errors: > > llvm-objcopy: error: --set-section-flags=.text conflicts with > > --rename-section=.text=.rodata > > > > Rather than support setting flags then renaming sections vs renaming > > then setting flags, it's simpler to just change both at the same time > > via --rename-section. > > > > This can be verified with: > > $ readelf -S drivers/misc/lkdtm/rodata_objcopy.o > > ... > > Section Headers: > > [Nr] Name Type Address Offset > > Size EntSize Flags Link Info Align > > ... > > [ 1] .rodata PROGBITS 0000000000000000 00000040 > > 0000000000000004 0000000000000000 A 0 0 4 > > ... > > > > Which shows in the Flags field that .text is now renamed .rodata, the > > append flag A is set, and the section is not flagged as writeable W. > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/448 > > Reported-by: Nathan Chancellor <nathanchance@gmail.com> > > This should be natechancellor@gmail.com (although I think I do own that > email, just haven't been into it for 10+ years...) > > > Suggested-by: Jordan Rupprect <rupprecht@google.com> > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > > --- > > drivers/misc/lkdtm/Makefile | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/drivers/misc/lkdtm/Makefile b/drivers/misc/lkdtm/Makefile > > index 951c984de61a..89dee2a9d88c 100644 > > --- a/drivers/misc/lkdtm/Makefile > > +++ b/drivers/misc/lkdtm/Makefile > > @@ -15,8 +15,7 @@ KCOV_INSTRUMENT_rodata.o := n > > > > OBJCOPYFLAGS := > > OBJCOPYFLAGS_rodata_objcopy.o := \ > > - --set-section-flags .text=alloc,readonly \ > > - --rename-section .text=.rodata > > + --rename-section .text=.rodata,alloc,readonly > > targets += rodata.o rodata_objcopy.o > > $(obj)/rodata_objcopy.o: $(obj)/rodata.o FORCE > > $(call if_changed,objcopy) > > -- > > 2.21.0.1020.gf2820cf01a-goog > > > > I ran this script to see if there was any change for GNU objcopy and it > looks like .rodata's type gets changed, is this intentional? Otherwise, > this works for llvm-objcopy like you show. > > ----------- > > 1c1 > < There are 11 section headers, starting at offset 0x240: > --- > > There are 11 section headers, starting at offset 0x230: > 8c8 > < [ 1] .rodata PROGBITS 0000000000000000 00000040 > --- > > [ 1] .rodata NOBITS 0000000000000000 00000040 > 10c10 > > ----------- > > #!/bin/bash > > TMP1=$(mktemp) > TMP2=$(mktemp) > > git checkout next-20190513 > > make -j$(nproc) ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- O=out mrproper allyesconfig drivers/misc/lkdtm/ > readelf -S out/drivers/misc/lkdtm/rodata_objcopy.o > ${TMP1} > > curl -LSs https://lore.kernel.org/lkml/20190513222109.110020-1-ndesaulniers@google.com/raw | git am -3 > > make -j$(nproc) ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- O=out mrproper allyesconfig drivers/misc/lkdtm/ > readelf -S out/drivers/misc/lkdtm/rodata_objcopy.o > ${TMP2} > > diff ${TMP1} ${TMP2} [-- Attachment #2: S/MIME Cryptographic Signature --] [-- Type: application/pkcs7-signature, Size: 4849 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] lkdtm: support llvm-objcopy 2019-05-13 23:38 ` Jordan Rupprecht @ 2019-05-13 23:47 ` Nathan Chancellor 0 siblings, 0 replies; 16+ messages in thread From: Nathan Chancellor @ 2019-05-13 23:47 UTC (permalink / raw) To: Jordan Rupprecht Cc: Nick Desaulniers, keescook, clang-built-linux, Nathan Chancellor, Arnd Bergmann, Greg Kroah-Hartman, LKML On Mon, May 13, 2019 at 04:38:54PM -0700, Jordan Rupprecht wrote: > Nathan: is your version of llvm-objcopy later than r359639 (April 30)? > > > From: Nathan Chancellor <natechancellor@gmail.com> > Date: Mon, May 13, 2019 at 4:29 PM > To: Nick Desaulniers > Cc: <keescook@chromium.org>, <clang-built-linux@googlegroups.com>, > Nathan Chancellor, Jordan Rupprect, Arnd Bergmann, Greg Kroah-Hartman, > <linux-kernel@vger.kernel.org> > > > On Mon, May 13, 2019 at 03:21:09PM -0700, 'Nick Desaulniers' via Clang Built Linux wrote: > > > With CONFIG_LKDTM=y and make OBJCOPY=llvm-objcopy, llvm-objcopy errors: > > > llvm-objcopy: error: --set-section-flags=.text conflicts with > > > --rename-section=.text=.rodata > > > > > > Rather than support setting flags then renaming sections vs renaming > > > then setting flags, it's simpler to just change both at the same time > > > via --rename-section. > > > > > > This can be verified with: > > > $ readelf -S drivers/misc/lkdtm/rodata_objcopy.o > > > ... > > > Section Headers: > > > [Nr] Name Type Address Offset > > > Size EntSize Flags Link Info Align > > > ... > > > [ 1] .rodata PROGBITS 0000000000000000 00000040 > > > 0000000000000004 0000000000000000 A 0 0 4 > > > ... > > > > > > Which shows in the Flags field that .text is now renamed .rodata, the > > > append flag A is set, and the section is not flagged as writeable W. > > > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/448 > > > Reported-by: Nathan Chancellor <nathanchance@gmail.com> > > > > This should be natechancellor@gmail.com (although I think I do own that > > email, just haven't been into it for 10+ years...) > > > > > Suggested-by: Jordan Rupprect <rupprecht@google.com> > > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > > > --- > > > drivers/misc/lkdtm/Makefile | 3 +-- > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > diff --git a/drivers/misc/lkdtm/Makefile b/drivers/misc/lkdtm/Makefile > > > index 951c984de61a..89dee2a9d88c 100644 > > > --- a/drivers/misc/lkdtm/Makefile > > > +++ b/drivers/misc/lkdtm/Makefile > > > @@ -15,8 +15,7 @@ KCOV_INSTRUMENT_rodata.o := n > > > > > > OBJCOPYFLAGS := > > > OBJCOPYFLAGS_rodata_objcopy.o := \ > > > - --set-section-flags .text=alloc,readonly \ > > > - --rename-section .text=.rodata > > > + --rename-section .text=.rodata,alloc,readonly > > > targets += rodata.o rodata_objcopy.o > > > $(obj)/rodata_objcopy.o: $(obj)/rodata.o FORCE > > > $(call if_changed,objcopy) > > > -- > > > 2.21.0.1020.gf2820cf01a-goog > > > > > > > I ran this script to see if there was any change for GNU objcopy and it > > looks like .rodata's type gets changed, is this intentional? Otherwise, > > this works for llvm-objcopy like you show. > > > > ----------- > > > > 1c1 > > < There are 11 section headers, starting at offset 0x240: > > --- > > > There are 11 section headers, starting at offset 0x230: > > 8c8 > > < [ 1] .rodata PROGBITS 0000000000000000 00000040 > > --- > > > [ 1] .rodata NOBITS 0000000000000000 00000040 > > 10c10 > > > > ----------- > > > > #!/bin/bash > > > > TMP1=$(mktemp) > > TMP2=$(mktemp) > > > > git checkout next-20190513 > > > > make -j$(nproc) ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- O=out mrproper allyesconfig drivers/misc/lkdtm/ > > readelf -S out/drivers/misc/lkdtm/rodata_objcopy.o > ${TMP1} > > > > curl -LSs https://lore.kernel.org/lkml/20190513222109.110020-1-ndesaulniers@google.com/raw | git am -3 > > > > make -j$(nproc) ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- O=out mrproper allyesconfig drivers/misc/lkdtm/ > > readelf -S out/drivers/misc/lkdtm/rodata_objcopy.o > ${TMP2} > > > > diff ${TMP1} ${TMP2} Hi Jordan, Yes but that output was purely with GNU objcopy (checking section headers before and after this patch). This is the section header for llvm-objcopy after this patch: [ 1] .rodata PROGBITS 0000000000000000 00000040 0000000000000004 0000000000000000 A 0 0 4 Cheers, Nathan ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] lkdtm: support llvm-objcopy 2019-05-13 23:29 ` Nathan Chancellor 2019-05-13 23:38 ` Jordan Rupprecht @ 2019-05-13 23:50 ` Nick Desaulniers 2019-05-14 18:10 ` Kees Cook 1 sibling, 1 reply; 16+ messages in thread From: Nick Desaulniers @ 2019-05-13 23:50 UTC (permalink / raw) To: Nathan Chancellor Cc: Kees Cook, clang-built-linux, Nathan Chancellor, Jordan Rupprect, Arnd Bergmann, Greg Kroah-Hartman, LKML On Mon, May 13, 2019 at 4:29 PM Nathan Chancellor <natechancellor@gmail.com> wrote: > > On Mon, May 13, 2019 at 03:21:09PM -0700, 'Nick Desaulniers' via Clang Built Linux wrote: > > With CONFIG_LKDTM=y and make OBJCOPY=llvm-objcopy, llvm-objcopy errors: > > llvm-objcopy: error: --set-section-flags=.text conflicts with > > --rename-section=.text=.rodata > > > > Rather than support setting flags then renaming sections vs renaming > > then setting flags, it's simpler to just change both at the same time > > via --rename-section. > > > > This can be verified with: > > $ readelf -S drivers/misc/lkdtm/rodata_objcopy.o > > ... > > Section Headers: > > [Nr] Name Type Address Offset > > Size EntSize Flags Link Info Align > > ... > > [ 1] .rodata PROGBITS 0000000000000000 00000040 > > 0000000000000004 0000000000000000 A 0 0 4 > > ... > > > > Which shows in the Flags field that .text is now renamed .rodata, the > > append flag A is set, and the section is not flagged as writeable W. > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/448 > > Reported-by: Nathan Chancellor <nathanchance@gmail.com> > > This should be natechancellor@gmail.com (although I think I do own that > email, just haven't been into it for 10+ years...) Sorry, I should have looked it up. I'll just fix this, my earlier mistake, and collect Kee's reviewed by tag in a v2 sent directly to GKH. > > > Suggested-by: Jordan Rupprect <rupprecht@google.com> > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > > --- > > drivers/misc/lkdtm/Makefile | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/drivers/misc/lkdtm/Makefile b/drivers/misc/lkdtm/Makefile > > index 951c984de61a..89dee2a9d88c 100644 > > --- a/drivers/misc/lkdtm/Makefile > > +++ b/drivers/misc/lkdtm/Makefile > > @@ -15,8 +15,7 @@ KCOV_INSTRUMENT_rodata.o := n > > > > OBJCOPYFLAGS := > > OBJCOPYFLAGS_rodata_objcopy.o := \ > > - --set-section-flags .text=alloc,readonly \ > > - --rename-section .text=.rodata > > + --rename-section .text=.rodata,alloc,readonly > > targets += rodata.o rodata_objcopy.o > > $(obj)/rodata_objcopy.o: $(obj)/rodata.o FORCE > > $(call if_changed,objcopy) > > -- > > 2.21.0.1020.gf2820cf01a-goog > > > > I ran this script to see if there was any change for GNU objcopy and it > looks like .rodata's type gets changed, is this intentional? Otherwise, > this works for llvm-objcopy like you show. > > ----------- > > 1c1 > < There are 11 section headers, starting at offset 0x240: > --- > > There are 11 section headers, starting at offset 0x230: > 8c8 > < [ 1] .rodata PROGBITS 0000000000000000 00000040 > --- > > [ 1] .rodata NOBITS 0000000000000000 00000040 > 10c10 Interesting find. the .rodata of vmlinux itself is marked PROGBITS, so its curious that GNU binutils changes the "Type" after the rename. I doubt the code in question relies on NOBITS for this section. Kees, can you clarify? Jordan, do you know what the differences are between PROGBITS vs NOBITS? https://people.redhat.com/mpolacek/src/devconf2012.pdf seems to suggest NOBITS zero initializes data but I'm not sure that's what this code wants. > > ----------- > > #!/bin/bash > > TMP1=$(mktemp) > TMP2=$(mktemp) > > git checkout next-20190513 > > make -j$(nproc) ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- O=out mrproper allyesconfig drivers/misc/lkdtm/ > readelf -S out/drivers/misc/lkdtm/rodata_objcopy.o > ${TMP1} > > curl -LSs https://lore.kernel.org/lkml/20190513222109.110020-1-ndesaulniers@google.com/raw | git am -3 > > make -j$(nproc) ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- O=out mrproper allyesconfig drivers/misc/lkdtm/ > readelf -S out/drivers/misc/lkdtm/rodata_objcopy.o > ${TMP2} > > diff ${TMP1} ${TMP2} -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] lkdtm: support llvm-objcopy 2019-05-13 23:50 ` Nick Desaulniers @ 2019-05-14 18:10 ` Kees Cook 2019-05-14 20:24 ` Nick Desaulniers 0 siblings, 1 reply; 16+ messages in thread From: Kees Cook @ 2019-05-14 18:10 UTC (permalink / raw) To: Nick Desaulniers Cc: Nathan Chancellor, clang-built-linux, Nathan Chancellor, Jordan Rupprect, Arnd Bergmann, Greg Kroah-Hartman, LKML On Mon, May 13, 2019 at 04:50:05PM -0700, Nick Desaulniers wrote: > On Mon, May 13, 2019 at 4:29 PM Nathan Chancellor > <natechancellor@gmail.com> wrote: > > > > On Mon, May 13, 2019 at 03:21:09PM -0700, 'Nick Desaulniers' via Clang Built Linux wrote: > > > With CONFIG_LKDTM=y and make OBJCOPY=llvm-objcopy, llvm-objcopy errors: > > > llvm-objcopy: error: --set-section-flags=.text conflicts with > > > --rename-section=.text=.rodata > > > > > > Rather than support setting flags then renaming sections vs renaming > > > then setting flags, it's simpler to just change both at the same time > > > via --rename-section. > > > > > > This can be verified with: > > > $ readelf -S drivers/misc/lkdtm/rodata_objcopy.o > > > ... > > > Section Headers: > > > [Nr] Name Type Address Offset > > > Size EntSize Flags Link Info Align > > > ... > > > [ 1] .rodata PROGBITS 0000000000000000 00000040 > > > 0000000000000004 0000000000000000 A 0 0 4 > > > ... > > > > > > Which shows in the Flags field that .text is now renamed .rodata, the > > > append flag A is set, and the section is not flagged as writeable W. > > > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/448 > > > Reported-by: Nathan Chancellor <nathanchance@gmail.com> > > > > This should be natechancellor@gmail.com (although I think I do own that > > email, just haven't been into it for 10+ years...) > > Sorry, I should have looked it up. I'll just fix this, my earlier > mistake, and collect Kees's reviewed by tag in a v2 sent directly to > GKH. Sounds good. > > I ran this script to see if there was any change for GNU objcopy and it > > looks like .rodata's type gets changed, is this intentional? Otherwise, > > this works for llvm-objcopy like you show. > > > > ----------- > > > > 1c1 > > < There are 11 section headers, starting at offset 0x240: > > --- > > > There are 11 section headers, starting at offset 0x230: > > 8c8 > > < [ 1] .rodata PROGBITS 0000000000000000 00000040 > > --- > > > [ 1] .rodata NOBITS 0000000000000000 00000040 > > 10c10 Oh, very good catch; thank you! > > Interesting find. the .rodata of vmlinux itself is marked PROGBITS, > so its curious that GNU binutils changes the "Type" after the rename. > I doubt the code in question relies on NOBITS for this section. Kees, > can you clarify? Jordan, do you know what the differences are between > PROGBITS vs NOBITS? > https://people.redhat.com/mpolacek/src/devconf2012.pdf seems to > suggest NOBITS zero initializes data but I'm not sure that's what this > code wants. Yes, the linker treats this as a zeroed section. My testing only checked that the NX protection check kicked, but in looking at the memory, the failure mode wouldn't have returned, because it got zeroed instead of seeing a "ret": Before the patch (with a two-byte target dump added): lkdtm: attempting bad execution at ffffffff986db2b0 lkdtm: f3 c3 After the patch: lkdtm: attempting bad execution at ffffffff986db2b0 lkdtm: 00 00 So, yes, this breaks the fall-back case and should not be used. It seems that objcopy BFD breaks the PROGBITS in this case, but llvm-objcopy does not... $ objcopy --set-section-flags .text=alloc,readonly --rename-section .text=.rodata rodata.o rodata_objcopy.o $ readelf -S rodata_objcopy.o | grep -A1 \.rodata [ 1] .rodata PROGBITS 0000000000000000 00000040 0000000000000002 0000000000000000 A 0 0 16 $ objcopy --rename-section .text=.rodata,alloc,readonly rodata.o rodata_objcopy.o $ readelf -S rodata_objcopy.o | grep -A1 \.rodata [ 1] .rodata NOBITS 0000000000000000 00000040 0000000000000002 0000000000000000 A 0 0 16 $ llvm-objcopy --rename-section .text=.rodata,alloc,readonly rodata.o rodata_objcopy-llvm.o $ readelf -S rodata_objcopy-llvm.o | grep -A1 \.rodata [ 1] .rodata PROGBITS 0000000000000000 00000040 0000000000000002 0000000000000000 A 0 0 16 llvm-objcopy doesn't like doing both arguments at the same time, and BFD gets it wrong when using the appended flags. How about just a two-stage copy, like this? diff --git a/drivers/misc/lkdtm/Makefile b/drivers/misc/lkdtm/Makefile index 951c984de61a..715832c844c8 100644 --- a/drivers/misc/lkdtm/Makefile +++ b/drivers/misc/lkdtm/Makefile @@ -14,9 +14,12 @@ KASAN_SANITIZE_stackleak.o := n KCOV_INSTRUMENT_rodata.o := n OBJCOPYFLAGS := +OBJCOPYFLAGS_rodata_flags.o := \ + --set-section-flags .text=alloc,readonly OBJCOPYFLAGS_rodata_objcopy.o := \ - --set-section-flags .text=alloc,readonly \ --rename-section .text=.rodata -targets += rodata.o rodata_objcopy.o -$(obj)/rodata_objcopy.o: $(obj)/rodata.o FORCE +targets += rodata.o rodata_flags.o rodata_objcopy.o +$(obj)/rodata_flags.o: $(obj)/rodata.o FORCE + $(call if_changed,objcopy) +$(obj)/rodata_objcopy.o: $(obj)/rodata_flags.o FORCE $(call if_changed,objcopy) -- Kees Cook ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] lkdtm: support llvm-objcopy 2019-05-14 18:10 ` Kees Cook @ 2019-05-14 20:24 ` Nick Desaulniers 2019-05-15 16:42 ` Kees Cook 0 siblings, 1 reply; 16+ messages in thread From: Nick Desaulniers @ 2019-05-14 20:24 UTC (permalink / raw) To: Kees Cook Cc: Nathan Chancellor, clang-built-linux, Nathan Chancellor, Jordan Rupprect, Arnd Bergmann, Greg Kroah-Hartman, LKML On Tue, May 14, 2019 at 11:11 AM Kees Cook <keescook@chromium.org> wrote: > > On Mon, May 13, 2019 at 04:50:05PM -0700, Nick Desaulniers wrote: > > On Mon, May 13, 2019 at 4:29 PM Nathan Chancellor > > <natechancellor@gmail.com> wrote: > > > > > > On Mon, May 13, 2019 at 03:21:09PM -0700, 'Nick Desaulniers' via Clang Built Linux wrote: > > > > With CONFIG_LKDTM=y and make OBJCOPY=llvm-objcopy, llvm-objcopy errors: > > > > llvm-objcopy: error: --set-section-flags=.text conflicts with > > > > --rename-section=.text=.rodata > > > > > > > > Rather than support setting flags then renaming sections vs renaming > > > > then setting flags, it's simpler to just change both at the same time > > > > via --rename-section. > > > > > > > > This can be verified with: > > > > $ readelf -S drivers/misc/lkdtm/rodata_objcopy.o > > > > ... > > > > Section Headers: > > > > [Nr] Name Type Address Offset > > > > Size EntSize Flags Link Info Align > > > > ... > > > > [ 1] .rodata PROGBITS 0000000000000000 00000040 > > > > 0000000000000004 0000000000000000 A 0 0 4 > > > > ... > > > > > > > > Which shows in the Flags field that .text is now renamed .rodata, the > > > > append flag A is set, and the section is not flagged as writeable W. > > > > > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/448 > > > > Reported-by: Nathan Chancellor <nathanchance@gmail.com> > > > > > > This should be natechancellor@gmail.com (although I think I do own that > > > email, just haven't been into it for 10+ years...) > > > > Sorry, I should have looked it up. I'll just fix this, my earlier > > mistake, and collect Kees's reviewed by tag in a v2 sent directly to > > GKH. > > Sounds good. > > > > I ran this script to see if there was any change for GNU objcopy and it > > > looks like .rodata's type gets changed, is this intentional? Otherwise, > > > this works for llvm-objcopy like you show. > > > > > > ----------- > > > > > > 1c1 > > > < There are 11 section headers, starting at offset 0x240: > > > --- > > > > There are 11 section headers, starting at offset 0x230: > > > 8c8 > > > < [ 1] .rodata PROGBITS 0000000000000000 00000040 > > > --- > > > > [ 1] .rodata NOBITS 0000000000000000 00000040 > > > 10c10 > > Oh, very good catch; thank you! > > > > > Interesting find. the .rodata of vmlinux itself is marked PROGBITS, > > so its curious that GNU binutils changes the "Type" after the rename. > > I doubt the code in question relies on NOBITS for this section. Kees, > > can you clarify? Jordan, do you know what the differences are between > > PROGBITS vs NOBITS? > > https://people.redhat.com/mpolacek/src/devconf2012.pdf seems to > > suggest NOBITS zero initializes data but I'm not sure that's what this > > code wants. > > Yes, the linker treats this as a zeroed section. My testing only checked that the NX protection check kicked, but in looking at the memory, the failure mode wouldn't have returned, because it got zeroed instead of seeing a "ret": > > Before the patch (with a two-byte target dump added): > > lkdtm: attempting bad execution at ffffffff986db2b0 > lkdtm: f3 c3 > > After the patch: > > lkdtm: attempting bad execution at ffffffff986db2b0 > lkdtm: 00 00 > > So, yes, this breaks the fall-back case and should not be used. It seems > that objcopy BFD breaks the PROGBITS in this case, but llvm-objcopy > does not... I'm not sure I want to call it a bug in the initial implementation. I've filed: https://sourceware.org/bugzilla/show_bug.cgi?id=24554 for clarification. Jordan, I hope you can participate in any discussion there? > > $ objcopy --set-section-flags .text=alloc,readonly --rename-section .text=.rodata rodata.o rodata_objcopy.o > $ readelf -S rodata_objcopy.o | grep -A1 \.rodata > [ 1] .rodata PROGBITS 0000000000000000 00000040 > 0000000000000002 0000000000000000 A 0 0 16 > > $ objcopy --rename-section .text=.rodata,alloc,readonly rodata.o rodata_objcopy.o > $ readelf -S rodata_objcopy.o | grep -A1 \.rodata > [ 1] .rodata NOBITS 0000000000000000 00000040 > 0000000000000002 0000000000000000 A 0 0 16 > > $ llvm-objcopy --rename-section .text=.rodata,alloc,readonly rodata.o rodata_objcopy-llvm.o > $ readelf -S rodata_objcopy-llvm.o | grep -A1 \.rodata > [ 1] .rodata PROGBITS 0000000000000000 00000040 > 0000000000000002 0000000000000000 A 0 0 16 > > > llvm-objcopy doesn't like doing both arguments at the same time, > and BFD gets it wrong when using the appended flags. How about just a > two-stage copy, like this? Yeah, I think this should work. What you wrote above + the link the bug I just filed would go well in a commit message, along with: Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > > diff --git a/drivers/misc/lkdtm/Makefile b/drivers/misc/lkdtm/Makefile > index 951c984de61a..715832c844c8 100644 > --- a/drivers/misc/lkdtm/Makefile > +++ b/drivers/misc/lkdtm/Makefile > @@ -14,9 +14,12 @@ KASAN_SANITIZE_stackleak.o := n > KCOV_INSTRUMENT_rodata.o := n > > OBJCOPYFLAGS := > +OBJCOPYFLAGS_rodata_flags.o := \ > + --set-section-flags .text=alloc,readonly > OBJCOPYFLAGS_rodata_objcopy.o := \ > - --set-section-flags .text=alloc,readonly \ > --rename-section .text=.rodata > -targets += rodata.o rodata_objcopy.o > -$(obj)/rodata_objcopy.o: $(obj)/rodata.o FORCE > +targets += rodata.o rodata_flags.o rodata_objcopy.o > +$(obj)/rodata_flags.o: $(obj)/rodata.o FORCE > + $(call if_changed,objcopy) > +$(obj)/rodata_objcopy.o: $(obj)/rodata_flags.o FORCE > $(call if_changed,objcopy) > > > -- > Kees Cook > > -- > You received this message because you are subscribed to the Google Groups "Clang Built Linux" group. > To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com. > To post to this group, send email to clang-built-linux@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/201905141041.C38DA1B305%40keescook. > For more options, visit https://groups.google.com/d/optout. -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] lkdtm: support llvm-objcopy 2019-05-14 20:24 ` Nick Desaulniers @ 2019-05-15 16:42 ` Kees Cook 2019-05-15 17:37 ` Nick Desaulniers 0 siblings, 1 reply; 16+ messages in thread From: Kees Cook @ 2019-05-15 16:42 UTC (permalink / raw) To: Nick Desaulniers Cc: Nathan Chancellor, clang-built-linux, Nathan Chancellor, Jordan Rupprect, Arnd Bergmann, Greg Kroah-Hartman, LKML On Tue, May 14, 2019 at 01:24:37PM -0700, Nick Desaulniers wrote: > On Tue, May 14, 2019 at 11:11 AM Kees Cook <keescook@chromium.org> wrote: > > > > On Mon, May 13, 2019 at 04:50:05PM -0700, Nick Desaulniers wrote: > > > On Mon, May 13, 2019 at 4:29 PM Nathan Chancellor > > > <natechancellor@gmail.com> wrote: > > > > > > > > On Mon, May 13, 2019 at 03:21:09PM -0700, 'Nick Desaulniers' via Clang Built Linux wrote: > > > > > With CONFIG_LKDTM=y and make OBJCOPY=llvm-objcopy, llvm-objcopy errors: > > > > > llvm-objcopy: error: --set-section-flags=.text conflicts with > > > > > --rename-section=.text=.rodata > > > > > > > > > > Rather than support setting flags then renaming sections vs renaming > > > > > then setting flags, it's simpler to just change both at the same time > > > > > via --rename-section. > > I'm not sure I want to call it a bug in the initial implementation. I've filed: > https://sourceware.org/bugzilla/show_bug.cgi?id=24554 for > clarification. Jordan, I hope you can participate in any discussion > there? Based on the hint from Alan Modra, it seems PROGBITS/NOBITS can be controlled with the presence/absence of the "load" section flag. This appears to work for both BFD and LLVM: $ objcopy --rename-section .text=.rodata,alloc,readonly,load rodata.o rodata_objcopy.o $ readelf -WS rodata_objcopy.o | grep \.rodata [ 1] .rodata PROGBITS 0000000000000000 000040 000002 00 A 0 0 16 $ llvm-objcopy --rename-section .text=.rodata,alloc,readonly,load rodata.o rodata_objcopy.o $ readelf -WS rodata_objcopy.o | grep \.rodata [ 1] .rodata PROGBITS 0000000000000000 000040 000002 00 A 0 0 16 So, that's an easy change that could be folded into the original version of this patch (no need for my two-phase work-around). -- Kees Cook ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] lkdtm: support llvm-objcopy 2019-05-15 16:42 ` Kees Cook @ 2019-05-15 17:37 ` Nick Desaulniers 2019-05-15 18:12 ` [PATCH v2] " Nick Desaulniers 0 siblings, 1 reply; 16+ messages in thread From: Nick Desaulniers @ 2019-05-15 17:37 UTC (permalink / raw) To: Kees Cook Cc: Nathan Chancellor, clang-built-linux, Nathan Chancellor, Jordan Rupprect, Arnd Bergmann, Greg Kroah-Hartman, LKML On Wed, May 15, 2019 at 9:43 AM Kees Cook <keescook@chromium.org> wrote: > > On Tue, May 14, 2019 at 01:24:37PM -0700, Nick Desaulniers wrote: > > On Tue, May 14, 2019 at 11:11 AM Kees Cook <keescook@chromium.org> wrote: > > > > > > On Mon, May 13, 2019 at 04:50:05PM -0700, Nick Desaulniers wrote: > > > > On Mon, May 13, 2019 at 4:29 PM Nathan Chancellor > > > > <natechancellor@gmail.com> wrote: > > > > > > > > > > On Mon, May 13, 2019 at 03:21:09PM -0700, 'Nick Desaulniers' via Clang Built Linux wrote: > > > > > > With CONFIG_LKDTM=y and make OBJCOPY=llvm-objcopy, llvm-objcopy errors: > > > > > > llvm-objcopy: error: --set-section-flags=.text conflicts with > > > > > > --rename-section=.text=.rodata > > > > > > > > > > > > Rather than support setting flags then renaming sections vs renaming > > > > > > then setting flags, it's simpler to just change both at the same time > > > > > > via --rename-section. > > > > I'm not sure I want to call it a bug in the initial implementation. I've filed: > > https://sourceware.org/bugzilla/show_bug.cgi?id=24554 for > > clarification. Jordan, I hope you can participate in any discussion > > there? > > Based on the hint from Alan Modra, it seems PROGBITS/NOBITS can be > controlled with the presence/absence of the "load" section flag. This > appears to work for both BFD and LLVM: > > $ objcopy --rename-section .text=.rodata,alloc,readonly,load rodata.o rodata_objcopy.o > $ readelf -WS rodata_objcopy.o | grep \.rodata > [ 1] .rodata PROGBITS 0000000000000000 000040 000002 00 A 0 0 16 > > $ llvm-objcopy --rename-section .text=.rodata,alloc,readonly,load rodata.o rodata_objcopy.o > $ readelf -WS rodata_objcopy.o | grep \.rodata > [ 1] .rodata PROGBITS 0000000000000000 000040 000002 00 A 0 0 16 > > So, that's an easy change that could be folded into the original version > of this patch (no need for my two-phase work-around). Ah, yes that's better. I'll fold that into a v2 and resend shortly. I'm going to carry your Ack from earlier, please let me know offlist if that's not appropriate. -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2] lkdtm: support llvm-objcopy 2019-05-15 17:37 ` Nick Desaulniers @ 2019-05-15 18:12 ` Nick Desaulniers 2019-05-15 18:19 ` Nathan Chancellor 0 siblings, 1 reply; 16+ messages in thread From: Nick Desaulniers @ 2019-05-15 18:12 UTC (permalink / raw) To: gregkh Cc: clang-built-linux, Nick Desaulniers, stable, Nathan Chancellor, Alan Modra, Jordan Rupprect, Kees Cook, Arnd Bergmann, linux-kernel With CONFIG_LKDTM=y and make OBJCOPY=llvm-objcopy, llvm-objcopy errors: llvm-objcopy: error: --set-section-flags=.text conflicts with --rename-section=.text=.rodata Rather than support setting flags then renaming sections vs renaming then setting flags, it's simpler to just change both at the same time via --rename-section. Adding the load flag is required for GNU objcopy to mark .rodata Type as PROGBITS after the rename. This can be verified with: $ readelf -S drivers/misc/lkdtm/rodata_objcopy.o ... Section Headers: [Nr] Name Type Address Offset Size EntSize Flags Link Info Align ... [ 1] .rodata PROGBITS 0000000000000000 00000040 0000000000000004 0000000000000000 A 0 0 4 ... Which shows that .text is now renamed .rodata, the alloc flag A is set, the type is PROGBITS, and the section is not flagged as writeable W. Cc: stable@vger.kernel.org Link: https://sourceware.org/bugzilla/show_bug.cgi?id=24554 Link: https://github.com/ClangBuiltLinux/linux/issues/448 Reported-by: Nathan Chancellor <nathanchance@gmail.com> Suggested-by: Alan Modra <amodra@gmail.com> Suggested-by: Jordan Rupprect <rupprecht@google.com> Suggested-by: Kees Cook <keescook@chromium.org> Acked-by: Kees Cook <keescook@chromium.org> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> --- Changes from v1 -> v2: * add load flag, as per Kees and Alan. * update commit message to mention reason for load flag. * add Kees' and Alan's suggested by. * carry Kees' Ack. * cc stable. drivers/misc/lkdtm/Makefile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/misc/lkdtm/Makefile b/drivers/misc/lkdtm/Makefile index 951c984de61a..fb10eafe9bde 100644 --- a/drivers/misc/lkdtm/Makefile +++ b/drivers/misc/lkdtm/Makefile @@ -15,8 +15,7 @@ KCOV_INSTRUMENT_rodata.o := n OBJCOPYFLAGS := OBJCOPYFLAGS_rodata_objcopy.o := \ - --set-section-flags .text=alloc,readonly \ - --rename-section .text=.rodata + --rename-section .text=.rodata,alloc,readonly,load targets += rodata.o rodata_objcopy.o $(obj)/rodata_objcopy.o: $(obj)/rodata.o FORCE $(call if_changed,objcopy) -- 2.21.0.1020.gf2820cf01a-goog ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2] lkdtm: support llvm-objcopy 2019-05-15 18:12 ` [PATCH v2] " Nick Desaulniers @ 2019-05-15 18:19 ` Nathan Chancellor 2019-05-15 18:24 ` [PATCH v3] " Nick Desaulniers 0 siblings, 1 reply; 16+ messages in thread From: Nathan Chancellor @ 2019-05-15 18:19 UTC (permalink / raw) To: Nick Desaulniers Cc: gregkh, clang-built-linux, stable, Nathan Chancellor, Alan Modra, Jordan Rupprect, Kees Cook, Arnd Bergmann, linux-kernel On Wed, May 15, 2019 at 11:12:04AM -0700, 'Nick Desaulniers' via Clang Built Linux wrote: > With CONFIG_LKDTM=y and make OBJCOPY=llvm-objcopy, llvm-objcopy errors: > llvm-objcopy: error: --set-section-flags=.text conflicts with > --rename-section=.text=.rodata > > Rather than support setting flags then renaming sections vs renaming > then setting flags, it's simpler to just change both at the same time > via --rename-section. Adding the load flag is required for GNU objcopy > to mark .rodata Type as PROGBITS after the rename. > > This can be verified with: > $ readelf -S drivers/misc/lkdtm/rodata_objcopy.o > ... > Section Headers: > [Nr] Name Type Address Offset > Size EntSize Flags Link Info Align > ... > [ 1] .rodata PROGBITS 0000000000000000 00000040 > 0000000000000004 0000000000000000 A 0 0 4 > ... > > Which shows that .text is now renamed .rodata, the alloc flag A is set, > the type is PROGBITS, and the section is not flagged as writeable W. > > Cc: stable@vger.kernel.org > Link: https://sourceware.org/bugzilla/show_bug.cgi?id=24554 > Link: https://github.com/ClangBuiltLinux/linux/issues/448 > Reported-by: Nathan Chancellor <nathanchance@gmail.com> Doesn't look like this got updated. I don't want to make you send a v3 just for that though since it's purely cosmetic and adding my tag below will ensure I get copied on any backports and such. > Suggested-by: Alan Modra <amodra@gmail.com> > Suggested-by: Jordan Rupprect <rupprecht@google.com> > Suggested-by: Kees Cook <keescook@chromium.org> > Acked-by: Kees Cook <keescook@chromium.org> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com> > --- > Changes from v1 -> v2: > * add load flag, as per Kees and Alan. > * update commit message to mention reason for load flag. > * add Kees' and Alan's suggested by. > * carry Kees' Ack. > * cc stable. > > drivers/misc/lkdtm/Makefile | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/misc/lkdtm/Makefile b/drivers/misc/lkdtm/Makefile > index 951c984de61a..fb10eafe9bde 100644 > --- a/drivers/misc/lkdtm/Makefile > +++ b/drivers/misc/lkdtm/Makefile > @@ -15,8 +15,7 @@ KCOV_INSTRUMENT_rodata.o := n > > OBJCOPYFLAGS := > OBJCOPYFLAGS_rodata_objcopy.o := \ > - --set-section-flags .text=alloc,readonly \ > - --rename-section .text=.rodata > + --rename-section .text=.rodata,alloc,readonly,load > targets += rodata.o rodata_objcopy.o > $(obj)/rodata_objcopy.o: $(obj)/rodata.o FORCE > $(call if_changed,objcopy) > -- > 2.21.0.1020.gf2820cf01a-goog > ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3] lkdtm: support llvm-objcopy 2019-05-15 18:19 ` Nathan Chancellor @ 2019-05-15 18:24 ` Nick Desaulniers [not found] ` <20190517001002.D1A262084A@mail.kernel.org> 0 siblings, 1 reply; 16+ messages in thread From: Nick Desaulniers @ 2019-05-15 18:24 UTC (permalink / raw) To: gregkh Cc: clang-built-linux, Nick Desaulniers, stable, Nathan Chancellor, Alan Modra, Jordan Rupprect, Kees Cook, Arnd Bergmann, linux-kernel With CONFIG_LKDTM=y and make OBJCOPY=llvm-objcopy, llvm-objcopy errors: llvm-objcopy: error: --set-section-flags=.text conflicts with --rename-section=.text=.rodata Rather than support setting flags then renaming sections vs renaming then setting flags, it's simpler to just change both at the same time via --rename-section. Adding the load flag is required for GNU objcopy to mark .rodata Type as PROGBITS after the rename. This can be verified with: $ readelf -S drivers/misc/lkdtm/rodata_objcopy.o ... Section Headers: [Nr] Name Type Address Offset Size EntSize Flags Link Info Align ... [ 1] .rodata PROGBITS 0000000000000000 00000040 0000000000000004 0000000000000000 A 0 0 4 ... Which shows that .text is now renamed .rodata, the alloc flag A is set, the type is PROGBITS, and the section is not flagged as writeable W. Cc: stable@vger.kernel.org Link: https://sourceware.org/bugzilla/show_bug.cgi?id=24554 Link: https://github.com/ClangBuiltLinux/linux/issues/448 Reported-by: Nathan Chancellor <natechancellor@gmail.com> Suggested-by: Alan Modra <amodra@gmail.com> Suggested-by: Jordan Rupprect <rupprecht@google.com> Suggested-by: Kees Cook <keescook@chromium.org> Acked-by: Kees Cook <keescook@chromium.org> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> --- Changes from v2 -> v3: * correct Nathan's email address and collect his reviewed by. Changes from v1 -> v2: * add load flag, as per Kees and Alan. * update commit message to mention reason for load flag. * add Kees' and Alan's suggested by. * carry Kees' Ack. * cc stable. drivers/misc/lkdtm/Makefile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/misc/lkdtm/Makefile b/drivers/misc/lkdtm/Makefile index 951c984de61a..fb10eafe9bde 100644 --- a/drivers/misc/lkdtm/Makefile +++ b/drivers/misc/lkdtm/Makefile @@ -15,8 +15,7 @@ KCOV_INSTRUMENT_rodata.o := n OBJCOPYFLAGS := OBJCOPYFLAGS_rodata_objcopy.o := \ - --set-section-flags .text=alloc,readonly \ - --rename-section .text=.rodata + --rename-section .text=.rodata,alloc,readonly,load targets += rodata.o rodata_objcopy.o $(obj)/rodata_objcopy.o: $(obj)/rodata.o FORCE $(call if_changed,objcopy) -- 2.21.0.1020.gf2820cf01a-goog ^ permalink raw reply related [flat|nested] 16+ messages in thread
[parent not found: <20190517001002.D1A262084A@mail.kernel.org>]
* Re: [PATCH v3] lkdtm: support llvm-objcopy [not found] ` <20190517001002.D1A262084A@mail.kernel.org> @ 2019-05-17 4:06 ` Nathan Chancellor 2019-05-17 4:09 ` Nathan Chancellor 0 siblings, 1 reply; 16+ messages in thread From: Nathan Chancellor @ 2019-05-17 4:06 UTC (permalink / raw) To: Sasha Levin; +Cc: Nick Desaulniers, gregkh, clang-built-linux, stable On Fri, May 17, 2019 at 12:10:02AM +0000, Sasha Levin wrote: > Hi, > > [This is an automated email] > > This commit has been processed because it contains a -stable tag. > The stable tag indicates that it's relevant for the following trees: all Nick, the stable tag should probably specify 4.8+ since the commit it fixes came in during 4.8-rc1. Cc: stable@vger.kernel.org # 4.8+ Might also help to add: Fixes: 9a49a528dcf3 ("lkdtm: add function for testing .rodata section") > > The bot has tested the following trees: v5.1.2, v5.0.16, v4.19.43, v4.14.119, v4.9.176, v4.4.179, v3.18.140. > > v5.1.2: Build OK! > v5.0.16: Build OK! > v4.19.43: Build OK! > v4.14.119: Failed to apply! Possible dependencies: > 039a1c42058d ("lkdtm: Relocate code to subdirectory") However, it will still need a manual backport because of the lack of this commit. I've attached the current patch backported for reference, the git am flags '-3 -p4 --directory=drivers/misc' help get it proper then the conflict is rather easy to resolve. Thanks, Nathan ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3] lkdtm: support llvm-objcopy 2019-05-17 4:06 ` Nathan Chancellor @ 2019-05-17 4:09 ` Nathan Chancellor 0 siblings, 0 replies; 16+ messages in thread From: Nathan Chancellor @ 2019-05-17 4:09 UTC (permalink / raw) To: Sasha Levin; +Cc: Nick Desaulniers, gregkh, clang-built-linux, stable [-- Attachment #1: Type: text/plain, Size: 1223 bytes --] On Thu, May 16, 2019 at 09:06:13PM -0700, Nathan Chancellor wrote: > On Fri, May 17, 2019 at 12:10:02AM +0000, Sasha Levin wrote: > > Hi, > > > > [This is an automated email] > > > > This commit has been processed because it contains a -stable tag. > > The stable tag indicates that it's relevant for the following trees: all > > Nick, the stable tag should probably specify 4.8+ since the commit it > fixes came in during 4.8-rc1. > > Cc: stable@vger.kernel.org # 4.8+ > > Might also help to add: > > Fixes: 9a49a528dcf3 ("lkdtm: add function for testing .rodata section") > > > > > The bot has tested the following trees: v5.1.2, v5.0.16, v4.19.43, v4.14.119, v4.9.176, v4.4.179, v3.18.140. > > > > v5.1.2: Build OK! > > v5.0.16: Build OK! > > v4.19.43: Build OK! > > v4.14.119: Failed to apply! Possible dependencies: > > 039a1c42058d ("lkdtm: Relocate code to subdirectory") > > However, it will still need a manual backport because of the lack of > this commit. I've attached the current patch backported for reference, Would help if I actually did this... :^) > the git am flags '-3 -p4 --directory=drivers/misc' help get it proper > then the conflict is rather easy to resolve. > > Thanks, > Nathan [-- Attachment #2: 0001-lkdtm-support-llvm-objcopy.patch --] [-- Type: text/plain, Size: 2363 bytes --] From 10c0a74a8d92669b91b66efe6b72550fc24e8e7d Mon Sep 17 00:00:00 2001 From: Nick Desaulniers <ndesaulniers@google.com> Date: Wed, 15 May 2019 11:24:41 -0700 Subject: [PATCH] lkdtm: support llvm-objcopy With CONFIG_LKDTM=y and make OBJCOPY=llvm-objcopy, llvm-objcopy errors: llvm-objcopy: error: --set-section-flags=.text conflicts with --rename-section=.text=.rodata Rather than support setting flags then renaming sections vs renaming then setting flags, it's simpler to just change both at the same time via --rename-section. Adding the load flag is required for GNU objcopy to mark .rodata Type as PROGBITS after the rename. This can be verified with: $ readelf -S drivers/misc/lkdtm/rodata_objcopy.o ... Section Headers: [Nr] Name Type Address Offset Size EntSize Flags Link Info Align ... [ 1] .rodata PROGBITS 0000000000000000 00000040 0000000000000004 0000000000000000 A 0 0 4 ... Which shows that .text is now renamed .rodata, the alloc flag A is set, the type is PROGBITS, and the section is not flagged as writeable W. Cc: stable@vger.kernel.org Link: https://sourceware.org/bugzilla/show_bug.cgi?id=24554 Link: https://github.com/ClangBuiltLinux/linux/issues/448 Reported-by: Nathan Chancellor <natechancellor@gmail.com> Suggested-by: Alan Modra <amodra@gmail.com> Suggested-by: Jordan Rupprect <rupprecht@google.com> Suggested-by: Kees Cook <keescook@chromium.org> Acked-by: Kees Cook <keescook@chromium.org> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> --- drivers/misc/Makefile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index c3c8624f4d95..805835c2b99b 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -70,8 +70,7 @@ KCOV_INSTRUMENT_lkdtm_rodata.o := n OBJCOPYFLAGS := OBJCOPYFLAGS_lkdtm_rodata_objcopy.o := \ - --set-section-flags .text=alloc,readonly \ - --rename-section .text=.rodata + --rename-section .text=.rodata,alloc,readonly,load targets += lkdtm_rodata.o lkdtm_rodata_objcopy.o $(obj)/lkdtm_rodata_objcopy.o: $(obj)/lkdtm_rodata.o FORCE $(call if_changed,objcopy) -- 2.22.0.rc0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
end of thread, other threads:[~2019-05-17 4:09 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-05-13 22:21 [PATCH] lkdtm: support llvm-objcopy Nick Desaulniers 2019-05-13 22:26 ` Nick Desaulniers 2019-05-13 23:04 ` Kees Cook 2019-05-13 23:29 ` Nathan Chancellor 2019-05-13 23:38 ` Jordan Rupprecht 2019-05-13 23:47 ` Nathan Chancellor 2019-05-13 23:50 ` Nick Desaulniers 2019-05-14 18:10 ` Kees Cook 2019-05-14 20:24 ` Nick Desaulniers 2019-05-15 16:42 ` Kees Cook 2019-05-15 17:37 ` Nick Desaulniers 2019-05-15 18:12 ` [PATCH v2] " Nick Desaulniers 2019-05-15 18:19 ` Nathan Chancellor 2019-05-15 18:24 ` [PATCH v3] " Nick Desaulniers [not found] ` <20190517001002.D1A262084A@mail.kernel.org> 2019-05-17 4:06 ` Nathan Chancellor 2019-05-17 4:09 ` Nathan Chancellor
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.