| Message ID | CAK8P3a3fHWdgH23LjACzM+htGCY-bHgM5Y6uB8J6K5XgfT8e+g@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Oct 17, 2017 at 8:52 AM, Arnd Bergmann <arnd@arndb.de> wrote: > On Tue, Oct 17, 2017 at 5:34 PM, Kees Cook <keescook@chromium.org> wrote: >> On Tue, Oct 17, 2017 at 8:26 AM, Kees Cook <keescook@chromium.org> wrote: >>> On Tue, Oct 17, 2017 at 8:23 AM, Arnd Bergmann <arnd@arndb.de> wrote: >>>> On Tue, Oct 17, 2017 at 1:00 PM, Arnd Bergmann <arnd@arndb.de> wrote: >>>>> Hi Kees, >>>>> >>>>> On my test box, current linux-next kernels fail to build due to the >>>>> patch that introduces CONFIG_CC_STACKPROTECTOR_AUTO, with my mainline >>>>> gcc >>>>> builds up to gcc-5.5.0. gcc-6 and higher work fine, as >>>>> scripts/gcc-x86_64-has-stack-protector.sh returns 'y' for those. >>>>> >>>>> Using the compilers provided by Ubuntu (4.6/4.7/4.8/4.9), everything >>>>> also works as expected, so my interpretation is that mainline gcc did >>>>> not enable the stack protector until gcc-6, while distributions did. >>>>> >>>>> Do you agree with that interpretation? >>>> >>>> It's probably a little different. I tried bisecting the gcc commit that fixed >>>> the issue for me, and ended up with this commit >>>> >>>> https://gitlab.indel.ch/thirdparty/gcc/commit/c14bac81551d6769741c2b1cc55e04d94fe8d3a7 >>>> >>>> that caused the target to change from x86_64-unknown-linux to >>>> x86_64-pc-linux, and apparently caused the compiler bootstrap >>>> to incorrectly identify the capabilities of the assembler. As a result, >>>> the assembler output inside of scripts/gcc-x86_64-has-stack-protector.sh >>>> that should be >>>> [snip] >>> >>> Yeah, %gs: vs __stack_chk_guard global. >>> >>> Do you know which gccs (of the past) had this? >>> >>> akpm's build error is different still, there are no warnings at all >>> and then the build fails with missing __stack_chks. I'm still trying >>> to figure that one out. >> >> Oh, I think I know what's happening. I'm going to try to simulate this >> and send another patch for testing... >> >> (I'm still curious about the compiler versions, since my gcc 4.4.4 >> works fine for stack-protector.) > > I've managed to reduce the change that fixed it to this bit in the > compiler sources: > > index dbfb978..d5bc694 100755 > --- a/config.guess > +++ b/config.guess > @@ -1021,7 +1021,7 @@ EOF > echo ${UNAME_MACHINE}-dec-linux-${LIBC} > exit ;; > x86_64:Linux:*:*) > - echo ${UNAME_MACHINE}-unknown-linux-${LIBC} > + echo ${UNAME_MACHINE}-pc-linux-${LIBC} > exit ;; > xtensa*:Linux:*:*) > echo ${UNAME_MACHINE}-unknown-linux-${LIBC} > > I still don't know why that makes a difference, but all versions > prior to gcc-6.1 have the problem for me. What happens if you add -mstack-protector-guard=tls ? -Kees
On Tue, Oct 17, 2017 at 8:26 PM, Kees Cook <keescook@chromium.org> wrote: > On Tue, Oct 17, 2017 at 8:52 AM, Arnd Bergmann <arnd@arndb.de> wrote: >> On Tue, Oct 17, 2017 at 5:34 PM, Kees Cook <keescook@chromium.org> wrote: >> index dbfb978..d5bc694 100755 >> --- a/config.guess >> +++ b/config.guess >> @@ -1021,7 +1021,7 @@ EOF >> echo ${UNAME_MACHINE}-dec-linux-${LIBC} >> exit ;; >> x86_64:Linux:*:*) >> - echo ${UNAME_MACHINE}-unknown-linux-${LIBC} >> + echo ${UNAME_MACHINE}-pc-linux-${LIBC} >> exit ;; >> xtensa*:Linux:*:*) >> echo ${UNAME_MACHINE}-unknown-linux-${LIBC} >> >> I still don't know why that makes a difference, but all versions >> prior to gcc-6.1 have the problem for me. > > What happens if you add -mstack-protector-guard=tls ? With gcc-4.8 an earlier, I get build failure: cc1: error: unrecognized command line option "-mstack-protector-guard=tls" With gcc-4.9 and gcc-5, I get this output: .file "" .text .globl foo .type foo, @function foo: pushq %rbp movq %rsp, %rbp subq $208, %rsp movq __stack_chk_guard(%rip), %rax movq %rax, -8(%rbp) xorl %eax, %eax movl $3, %eax movq -8(%rbp), %rdx xorq __stack_chk_guard(%rip), %rdx je .L3 call __stack_chk_fail .L3: leave ret .size foo, .-foo .ident "GCC: (GNU) 5.4.1 20170626" .section .note.GNU-stack,"",@progbits Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Oct 17, 2017 at 8:41 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Tue, Oct 17, 2017 at 8:26 PM, Kees Cook <keescook@chromium.org> wrote: >> On Tue, Oct 17, 2017 at 8:52 AM, Arnd Bergmann <arnd@arndb.de> wrote: >>> On Tue, Oct 17, 2017 at 5:34 PM, Kees Cook <keescook@chromium.org> wrote: > >>> index dbfb978..d5bc694 100755 >>> --- a/config.guess >>> +++ b/config.guess >>> @@ -1021,7 +1021,7 @@ EOF >>> echo ${UNAME_MACHINE}-dec-linux-${LIBC} >>> exit ;; >>> x86_64:Linux:*:*) >>> - echo ${UNAME_MACHINE}-unknown-linux-${LIBC} >>> + echo ${UNAME_MACHINE}-pc-linux-${LIBC} >>> exit ;; >>> xtensa*:Linux:*:*) >>> echo ${UNAME_MACHINE}-unknown-linux-${LIBC} >>> >>> I still don't know why that makes a difference, but all versions >>> prior to gcc-6.1 have the problem for me. >> >> What happens if you add -mstack-protector-guard=tls ? > > With gcc-4.8 an earlier, I get build failure: > > cc1: error: unrecognized command line option "-mstack-protector-guard=tls" > > With gcc-4.9 and gcc-5, I get this output: > To clarify: with my gcc-4.9/gcc-5 build, -mstack-protector-guard=tls has no effect, the output is the same as with -mstack-protector-guard=global using the Ubuntu compilers of the same version. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Oct 17, 2017 at 08:47:06PM +0200, Arnd Bergmann wrote: > To clarify: with my gcc-4.9/gcc-5 build, -mstack-protector-guard=tls > has no effect, > the output is the same as with -mstack-protector-guard=global using the Ubuntu > compilers of the same version. Jumping in here... on IRC Arnd suggested reverting 123c48cf899d ("Makefile: introduce CONFIG_CC_STACKPROTECTOR_AUTO") from -next. What do you think Kees?
On Tue, Oct 17, 2017 at 11:52 AM, Mark Brown <broonie@kernel.org> wrote: > On Tue, Oct 17, 2017 at 08:47:06PM +0200, Arnd Bergmann wrote: > >> To clarify: with my gcc-4.9/gcc-5 build, -mstack-protector-guard=tls >> has no effect, >> the output is the same as with -mstack-protector-guard=global using the Ubuntu >> compilers of the same version. > > Jumping in here... on IRC Arnd suggested reverting 123c48cf899d > ("Makefile: introduce CONFIG_CC_STACKPROTECTOR_AUTO") from -next. What > do you think Kees? Until we sort this out, yes, agreed. Andrew, can you pull the patches? -Kees
On Tue, Oct 17, 2017 at 11:47 AM, Arnd Bergmann <arnd@arndb.de> wrote: > On Tue, Oct 17, 2017 at 8:41 PM, Arnd Bergmann <arnd@arndb.de> wrote: >> On Tue, Oct 17, 2017 at 8:26 PM, Kees Cook <keescook@chromium.org> wrote: >>> On Tue, Oct 17, 2017 at 8:52 AM, Arnd Bergmann <arnd@arndb.de> wrote: >>>> On Tue, Oct 17, 2017 at 5:34 PM, Kees Cook <keescook@chromium.org> wrote: >> >>>> index dbfb978..d5bc694 100755 >>>> --- a/config.guess >>>> +++ b/config.guess >>>> @@ -1021,7 +1021,7 @@ EOF >>>> echo ${UNAME_MACHINE}-dec-linux-${LIBC} >>>> exit ;; >>>> x86_64:Linux:*:*) >>>> - echo ${UNAME_MACHINE}-unknown-linux-${LIBC} >>>> + echo ${UNAME_MACHINE}-pc-linux-${LIBC} >>>> exit ;; >>>> xtensa*:Linux:*:*) >>>> echo ${UNAME_MACHINE}-unknown-linux-${LIBC} >>>> >>>> I still don't know why that makes a difference, but all versions >>>> prior to gcc-6.1 have the problem for me. >>> >>> What happens if you add -mstack-protector-guard=tls ? >> >> With gcc-4.8 an earlier, I get build failure: >> >> cc1: error: unrecognized command line option "-mstack-protector-guard=tls" >> >> With gcc-4.9 and gcc-5, I get this output: >> > > To clarify: with my gcc-4.9/gcc-5 build, -mstack-protector-guard=tls > has no effect, > the output is the same as with -mstack-protector-guard=global using the Ubuntu > compilers of the same version. Er, okay. So, if -mstack-protector-guard=tls is recognized and produces references to __stack_chk_guard, something is extremely wrong. Stack protector works correctly for me on all the gccs I have, include the stock builds. $ gcc --version gcc (Ubuntu 6.3.0-12ubuntu2) 6.3.0 20170406 ... $ echo "int foo(void) { char X[200]; return 3; }" | gcc -S -x c -c -O0 -mcmodel=kernel -fno-PIE -fstack-protector - -o - ... movq %gs:40, %rax ... $ echo "int foo(void) { char X[200]; return 3; }" | gcc -mstack-protector-guard=tls -S -x c -c -O0 -mcmodel=kernel -fno-PIE -fstack-protector - -o - ... movq %gs:40, %rax ... $ echo "int foo(void) { char X[200]; return 3; }" | gcc -mstack-protector-guard=global -S -x c -c -O0 -mcmodel=kernel -fno-PIE -fstack-protector - -o - ... movq __stack_chk_guard(%rip), %rax ... $ ~/bin/gcc-4.8/gcc --version gcc (GCC) 4.8.5 ... $ echo "int foo(void) { char X[200]; return 3; }" | ~/bin/gcc-4.8/gcc -S -x c -c -O0 -mcmodel=kernel -fno-PIE -fstack-protector - -o - ... movq %gs:40, %rax ... $ ~/bin/gcc-4.7/gcc --version gcc (GCC) 4.7.4 ... $ echo "int foo(void) { char X[200]; return 3; }" | ~/bin/gcc-4.7/gcc -S -x c -c -O0 -mcmodel=kernel -fno-PIE -fstack-protector - -o - ... movq %gs:40, %rax ... $ ~/bin/gcc-4.4/gcc --version gcc (GCC) 4.4.4 ... $ echo "int foo(void) { char X[200]; return 3; }" | ~/bin/gcc-4.4/gcc -S -x c -c -O0 -mcmodel=kernel -fno-PIE -fstack-protector - -o - ... movq %gs:40, %rax ... -Kees
On Tue, 17 Oct 2017 11:53:10 -0700 Kees Cook <keescook@chromium.org> wrote: > On Tue, Oct 17, 2017 at 11:52 AM, Mark Brown <broonie@kernel.org> wrote: > > On Tue, Oct 17, 2017 at 08:47:06PM +0200, Arnd Bergmann wrote: > > > >> To clarify: with my gcc-4.9/gcc-5 build, -mstack-protector-guard=tls > >> has no effect, > >> the output is the same as with -mstack-protector-guard=global using the Ubuntu > >> compilers of the same version. > > > > Jumping in here... on IRC Arnd suggested reverting 123c48cf899d > > ("Makefile: introduce CONFIG_CC_STACKPROTECTOR_AUTO") from -next. What > > do you think Kees? > > Until we sort this out, yes, agreed. Andrew, can you pull the patches? Sure. All these? sh-boot-add-static-stack-protector-to-pre-kernel.patch makefile-move-stackprotector-availability-out-of-kconfig.patch makefile-introduce-config_cc_stackprotector_auto.patch makefile-introduce-config_cc_stackprotector_auto-fix.patch makefile-introduce-config_cc_stackprotector_auto-fix-2.patch makefile-introduce-config_cc_stackprotector_auto-fix-3.patch Mark, can you please drop those from -next also? -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Oct 17, 2017 at 12:56:45PM -0700, Andrew Morton wrote: > On Tue, 17 Oct 2017 11:53:10 -0700 Kees Cook <keescook@chromium.org> wrote: > > Until we sort this out, yes, agreed. Andrew, can you pull the patches? > Sure. All these? > sh-boot-add-static-stack-protector-to-pre-kernel.patch That one is still there as I'd already done the drop (after a conversation with Kees and Arnd on IRC). > makefile-move-stackprotector-availability-out-of-kconfig.patch > makefile-introduce-config_cc_stackprotector_auto.patch > makefile-introduce-config_cc_stackprotector_auto-fix.patch > makefile-introduce-config_cc_stackprotector_auto-fix-2.patch > makefile-introduce-config_cc_stackprotector_auto-fix-3.patch > Mark, can you please drop those from -next also? Hrm, I've got a horrible feeling I'm missing an update as I didn't drop or seem to have -3. Perhaps just a race with when I did the fetch. In any case none of those patches should be in there today assuming I've not gone and done yet more stupid stuff.
On Tue, Oct 17, 2017 at 12:56 PM, Andrew Morton <akpm@linux-foundation.org> wrote: > On Tue, 17 Oct 2017 11:53:10 -0700 Kees Cook <keescook@chromium.org> wrote: > >> On Tue, Oct 17, 2017 at 11:52 AM, Mark Brown <broonie@kernel.org> wrote: >> > On Tue, Oct 17, 2017 at 08:47:06PM +0200, Arnd Bergmann wrote: >> > >> >> To clarify: with my gcc-4.9/gcc-5 build, -mstack-protector-guard=tls >> >> has no effect, >> >> the output is the same as with -mstack-protector-guard=global using the Ubuntu >> >> compilers of the same version. >> > >> > Jumping in here... on IRC Arnd suggested reverting 123c48cf899d >> > ("Makefile: introduce CONFIG_CC_STACKPROTECTOR_AUTO") from -next. What >> > do you think Kees? >> >> Until we sort this out, yes, agreed. Andrew, can you pull the patches? > > Sure. All these? > > sh-boot-add-static-stack-protector-to-pre-kernel.patch > makefile-move-stackprotector-availability-out-of-kconfig.patch This one can stay. (It does actually fix another case no one else noticed.) > makefile-introduce-config_cc_stackprotector_auto.patch > makefile-introduce-config_cc_stackprotector_auto-fix.patch > makefile-introduce-config_cc_stackprotector_auto-fix-2.patch > makefile-introduce-config_cc_stackprotector_auto-fix-3.patch Yes, these should get dropped for the moment, thanks. Arnd and I have been trying to get to the bottom of it. -Kees
Hi Kees, 2017-10-18 5:06 GMT+09:00 Kees Cook <keescook@chromium.org>: > On Tue, Oct 17, 2017 at 12:56 PM, Andrew Morton > <akpm@linux-foundation.org> wrote: >> On Tue, 17 Oct 2017 11:53:10 -0700 Kees Cook <keescook@chromium.org> wrote: >> >>> On Tue, Oct 17, 2017 at 11:52 AM, Mark Brown <broonie@kernel.org> wrote: >>> > On Tue, Oct 17, 2017 at 08:47:06PM +0200, Arnd Bergmann wrote: >>> > >>> >> To clarify: with my gcc-4.9/gcc-5 build, -mstack-protector-guard=tls >>> >> has no effect, >>> >> the output is the same as with -mstack-protector-guard=global using the Ubuntu >>> >> compilers of the same version. >>> > >>> > Jumping in here... on IRC Arnd suggested reverting 123c48cf899d >>> > ("Makefile: introduce CONFIG_CC_STACKPROTECTOR_AUTO") from -next. What >>> > do you think Kees? >>> >>> Until we sort this out, yes, agreed. Andrew, can you pull the patches? >> >> Sure. All these? >> >> sh-boot-add-static-stack-protector-to-pre-kernel.patch >> makefile-move-stackprotector-availability-out-of-kconfig.patch > > This one can stay. (It does actually fix another case no one else noticed.) > >> makefile-introduce-config_cc_stackprotector_auto.patch >> makefile-introduce-config_cc_stackprotector_auto-fix.patch >> makefile-introduce-config_cc_stackprotector_auto-fix-2.patch >> makefile-introduce-config_cc_stackprotector_auto-fix-3.patch > > Yes, these should get dropped for the moment, thanks. Arnd and I have > been trying to get to the bottom of it. > > -Kees I see this series is repeating apply/drop, so I am not tracking down the latest status. When you have a chance for re-spin, please replace KBUILD_CFLAGS += -DCONFIG_CC_STACKPROTECTOR KBUILD_AFLAGS += -DCONFIG_CC_STACKPROTECTOR with KBUILD_CPPFLAGS += -DCONFIG_CC_STACKPROTECTOR
On Wed, Oct 18, 2017 at 5:41 PM, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > Hi Kees, > > 2017-10-18 5:06 GMT+09:00 Kees Cook <keescook@chromium.org>: >> On Tue, Oct 17, 2017 at 12:56 PM, Andrew Morton >> <akpm@linux-foundation.org> wrote: >>> On Tue, 17 Oct 2017 11:53:10 -0700 Kees Cook <keescook@chromium.org> wrote: >>> >>>> On Tue, Oct 17, 2017 at 11:52 AM, Mark Brown <broonie@kernel.org> wrote: >>>> > On Tue, Oct 17, 2017 at 08:47:06PM +0200, Arnd Bergmann wrote: >>>> > >>>> >> To clarify: with my gcc-4.9/gcc-5 build, -mstack-protector-guard=tls >>>> >> has no effect, >>>> >> the output is the same as with -mstack-protector-guard=global using the Ubuntu >>>> >> compilers of the same version. >>>> > >>>> > Jumping in here... on IRC Arnd suggested reverting 123c48cf899d >>>> > ("Makefile: introduce CONFIG_CC_STACKPROTECTOR_AUTO") from -next. What >>>> > do you think Kees? >>>> >>>> Until we sort this out, yes, agreed. Andrew, can you pull the patches? >>> >>> Sure. All these? >>> >>> sh-boot-add-static-stack-protector-to-pre-kernel.patch >>> makefile-move-stackprotector-availability-out-of-kconfig.patch >> >> This one can stay. (It does actually fix another case no one else noticed.) >> >>> makefile-introduce-config_cc_stackprotector_auto.patch >>> makefile-introduce-config_cc_stackprotector_auto-fix.patch >>> makefile-introduce-config_cc_stackprotector_auto-fix-2.patch >>> makefile-introduce-config_cc_stackprotector_auto-fix-3.patch >> >> Yes, these should get dropped for the moment, thanks. Arnd and I have >> been trying to get to the bottom of it. >> >> -Kees > > > I see this series is repeating apply/drop, > so I am not tracking down the latest status. They're currently dropped from -next while I figure out what's breaking 0-day (I haven't had a chance to track it down yet). > When you have a chance for re-spin, > please replace > > KBUILD_CFLAGS += -DCONFIG_CC_STACKPROTECTOR > KBUILD_AFLAGS += -DCONFIG_CC_STACKPROTECTOR > > with > > KBUILD_CPPFLAGS += -DCONFIG_CC_STACKPROTECTOR Ah! Sure. For some reason I thought CPPFLAGS weren't include in .S builds. -Kees
--- a/config.guess +++ b/config.guess @@ -1021,7 +1021,7 @@ EOF echo ${UNAME_MACHINE}-dec-linux-${LIBC} exit ;; x86_64:Linux:*:*) - echo ${UNAME_MACHINE}-unknown-linux-${LIBC} + echo ${UNAME_MACHINE}-pc-linux-${LIBC} exit ;; xtensa*:Linux:*:*) echo ${UNAME_MACHINE}-unknown-linux-${LIBC}