| Message ID | 20200506195138.22086-1-broonie@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | arm64: BTI kernel and vDSO support | expand |
On Wed, May 06, 2020 at 08:51:27PM +0100, Mark Brown wrote: > This patch series adds support for protecting the kernel and vDSO with > BTI including code compiled with the BPF JIT at runtime. > > We build the kernel with annotations for BTI and then map the kernel > with GP based on the support on the boot CPU, rejecting secondaries that > don't have BTI support. If there is a need to handle big.LITTLE systems > with mismatched BTI support we will have to revisit this, currently no > such implementations exist. > > This series depends on several branches in the arm64 tree: > > - for-next/bti-user > - for-next/insn > - for-next/asm > > v3: > - Add a patch adding a comment about why we enable leaf support for > PAC. > - Fix build of the 32 bit vDSO. > - Refactor the macro for emitting the ELF note for BTI code so that > the flags are defined separately in order to make it easier to > add handling for any future users. Bugger, I'm still getting warnings (clang 11.0.1), but from an allmodconfig build now: warning: some functions compiled with BTI and some compiled without BTI warning: not setting BTI in feature flags (repeated many, many times). I'll try to get you some more info. Will
On Thu, May 07, 2020 at 03:33:32PM +0100, Will Deacon wrote: > On Wed, May 06, 2020 at 08:51:27PM +0100, Mark Brown wrote: > > This patch series adds support for protecting the kernel and vDSO with > > BTI including code compiled with the BPF JIT at runtime. > > > > We build the kernel with annotations for BTI and then map the kernel > > with GP based on the support on the boot CPU, rejecting secondaries that > > don't have BTI support. If there is a need to handle big.LITTLE systems > > with mismatched BTI support we will have to revisit this, currently no > > such implementations exist. > > > > This series depends on several branches in the arm64 tree: > > > > - for-next/bti-user > > - for-next/insn > > - for-next/asm > > > > v3: > > - Add a patch adding a comment about why we enable leaf support for > > PAC. > > - Fix build of the 32 bit vDSO. > > - Refactor the macro for emitting the ELF note for BTI code so that > > the flags are defined separately in order to make it easier to > > add handling for any future users. > > Bugger, I'm still getting warnings (clang 11.0.1), but from an allmodconfig > build now: > > warning: some functions compiled with BTI and some compiled without BTI > warning: not setting BTI in feature flags > > (repeated many, many times). > > I'll try to get you some more info. Quick look at the log suggests that these are caused by HDRTEST, whatever that is. Will
On Thu, May 07, 2020 at 03:35:47PM +0100, Will Deacon wrote: > On Thu, May 07, 2020 at 03:33:32PM +0100, Will Deacon wrote: > > On Wed, May 06, 2020 at 08:51:27PM +0100, Mark Brown wrote: > > > This patch series adds support for protecting the kernel and vDSO with > > > BTI including code compiled with the BPF JIT at runtime. > > > > > > We build the kernel with annotations for BTI and then map the kernel > > > with GP based on the support on the boot CPU, rejecting secondaries that > > > don't have BTI support. If there is a need to handle big.LITTLE systems > > > with mismatched BTI support we will have to revisit this, currently no > > > such implementations exist. > > > > > > This series depends on several branches in the arm64 tree: > > > > > > - for-next/bti-user > > > - for-next/insn > > > - for-next/asm > > > > > > v3: > > > - Add a patch adding a comment about why we enable leaf support for > > > PAC. > > > - Fix build of the 32 bit vDSO. > > > - Refactor the macro for emitting the ELF note for BTI code so that > > > the flags are defined separately in order to make it easier to > > > add handling for any future users. > > > > Bugger, I'm still getting warnings (clang 11.0.1), but from an allmodconfig > > build now: > > > > warning: some functions compiled with BTI and some compiled without BTI > > warning: not setting BTI in feature flags > > > > (repeated many, many times). > > > > I'll try to get you some more info. > > Quick look at the log suggests that these are caused by HDRTEST, whatever > that is. Sorry, my parallel build threw me off, so I don't think it's HDRTEST after all. One of the problems appears to be tracing: CC kernel/trace/trace_clock.o warning: some functions compiled with BTI and some compiled without BTI warning: not setting BTI in feature flags Looking at objdump, there are funny gcov functions with PACISAP prologues: 0000000000023e40 <__llvm_gcov_writeout>: 23e40: a9be57fe stp x30, x21, [sp, #-32]! 23e44: a9014ff4 stp x20, x19, [sp, #16] 23e48: 94000000 bl 0 <__sanitizer_cov_trace_pc> 23e4c: 90000000 adrp x0, 0 <__register_ftrace_function> 23e50: 90000001 adrp x1, 0 <__register_ftrace_function> 23e54: 5293ac02 mov w2, #0x9d60 // #40288 23e58: 91000000 add x0, x0, #0x0 [...] :/ Will
On Thu, May 07, 2020 at 03:35:47PM +0100, Will Deacon wrote: > On Thu, May 07, 2020 at 03:33:32PM +0100, Will Deacon wrote: > > Bugger, I'm still getting warnings (clang 11.0.1), but from an allmodconfig > > build now: > > warning: some functions compiled with BTI and some compiled without BTI > > warning: not setting BTI in feature flags > > (repeated many, many times). > > I'll try to get you some more info. Where are you getting this clang from? When I test using clang 11 from the LLVM apt repos right now it seems fine, and clang 11 doesn't seem to have been released yet so I'm guessing this is some kind of snapshot. > Quick look at the log suggests that these are caused by HDRTEST, whatever > that is. AFAICT that's something that's supposed to be checking for include guards and not doing anything fancy... I can't think what could cause this on a per-function basis.
On Thu, May 07, 2020 at 03:59:01PM +0100, Will Deacon wrote: > Sorry, my parallel build threw me off, so I don't think it's HDRTEST after > all. One of the problems appears to be tracing: > > CC kernel/trace/trace_clock.o > warning: some functions compiled with BTI and some compiled without BTI > warning: not setting BTI in feature flags Can you share a .config?
On Thu, May 07, 2020 at 04:09:06PM +0100, Mark Brown wrote: > On Thu, May 07, 2020 at 03:59:01PM +0100, Will Deacon wrote: > > > Sorry, my parallel build threw me off, so I don't think it's HDRTEST after > > all. One of the problems appears to be tracing: > > > > CC kernel/trace/trace_clock.o > > warning: some functions compiled with BTI and some compiled without BTI > > warning: not setting BTI in feature flags > > Can you share a .config? allmodconfig Will
On Thu, May 07, 2020 at 04:07:13PM +0100, Mark Brown wrote: > On Thu, May 07, 2020 at 03:35:47PM +0100, Will Deacon wrote: > > On Thu, May 07, 2020 at 03:33:32PM +0100, Will Deacon wrote: > > > > Bugger, I'm still getting warnings (clang 11.0.1), but from an allmodconfig > > > build now: > > > > warning: some functions compiled with BTI and some compiled without BTI > > > warning: not setting BTI in feature flags > > > > (repeated many, many times). > > > > I'll try to get you some more info. > > Where are you getting this clang from? When I test using clang 11 from > the LLVM apt repos right now it seems fine, and clang 11 doesn't seem to > have been released yet so I'm guessing this is some kind of snapshot. I'm using a build of: https://android.googlesource.com/toolchain/llvm-project/+/b397f81060ce6d701042b782172ed13bee898b79 > > Quick look at the log suggests that these are caused by HDRTEST, whatever > > that is. > > AFAICT that's something that's supposed to be checking for include > guards and not doing anything fancy... I can't think what could cause > this on a per-function basis. Indeed, sorry for jumping the gun on that diagnosis. Will
On Thu, May 07, 2020 at 04:18:48PM +0100, Will Deacon wrote: > On Thu, May 07, 2020 at 04:09:06PM +0100, Mark Brown wrote: > > Can you share a .config? > allmodconfig Right, I'm seeing it here now - it's when CONFIG_GCOV_KERNEL is enabled and happens for clang-10 as well but not a GCC 10 prerelease build. Adding depends !(CC_IS_CLANG && GCOV_KERNEL) avoids the warning but obviously we should actually fix the issue.
On Thu, May 07, 2020 at 04:48:54PM +0100, Mark Brown wrote: > On Thu, May 07, 2020 at 04:18:48PM +0100, Will Deacon wrote: > > On Thu, May 07, 2020 at 04:09:06PM +0100, Mark Brown wrote: > > > > Can you share a .config? > > > allmodconfig > > Right, I'm seeing it here now - it's when CONFIG_GCOV_KERNEL is enabled > and happens for clang-10 as well but not a GCC 10 prerelease build. Interesting. Is that because GCC doesn't emit out-of-line GCOV functions, or does it emit PAC/BTI instructions for them instead? (you can disassemble one of the problematic opjects to have a look). > Adding > > depends !(CC_IS_CLANG && GCOV_KERNEL) > > avoids the warning but obviously we should actually fix the issue. I can't immediately see how to fix it, so your hack above might be the best bet for now. I'm just a little wary that it might not be limited to GCOV, but rather anything where the compiler provides a form of runtime. Anyway, I'll try your hack and see if I run into any more issues. Cheers, Will
On Thu, May 07, 2020 at 04:55:24PM +0100, Will Deacon wrote: > On Thu, May 07, 2020 at 04:48:54PM +0100, Mark Brown wrote: > > Right, I'm seeing it here now - it's when CONFIG_GCOV_KERNEL is enabled > > and happens for clang-10 as well but not a GCC 10 prerelease build. > Interesting. Is that because GCC doesn't emit out-of-line GCOV functions, > or does it emit PAC/BTI instructions for them instead? (you can disassemble > one of the problematic opjects to have a look). GCC does emit some helper functions wrapping GCOV stuff but they have appropriate annotations, eg: 00000000000000ac <_sub_D_00100_1>: ac: d503245f bti c b0: a9bf7bfd stp x29, x30, [sp, #-16]! b4: 910003fd mov x29, sp b8: 94000000 bl 0 <__gcov_exit> bc: a8c17bfd ldp x29, x30, [sp], #16 c0: d65f03c0 ret I can also reproduce this for clang with a trivial standalone source file and -fprofile-arcs -mbranch-protection=bti so it's nothing funky the kernel is doing as far as I can see. > I can't immediately see how to fix it, so your hack above might be the best > bet for now. I'm just a little wary that it might not be limited to GCOV, > but rather anything where the compiler provides a form of runtime. Indeed. I guess the nice thing with BTI is that if something goes wrong it will do so rather visibly so unless there are situations where the toolchain emits rarely called functions the problems will tend to be very obvious, and it seems that clang is detecting the problem itself and complaining loudly which makes it even more likely that if something else is affected it'll be noticed and we can at least add similar bodges. It does seem it's a straight compiler issue, if the compiler is emitting runtime then the compiler ought to be ensuring that it agrees with the build options the compiler was given and I can't think how this would be fixable or avoidable outside of the compiler other than "don't do that" which is what my Kconfig bodge did. I'm talking to the toolchain people internally about this.
On Thu, May 07, 2020 at 05:30:45PM +0100, Mark Brown wrote: > On Thu, May 07, 2020 at 04:55:24PM +0100, Will Deacon wrote: > > On Thu, May 07, 2020 at 04:48:54PM +0100, Mark Brown wrote: > > > > Right, I'm seeing it here now - it's when CONFIG_GCOV_KERNEL is enabled > > > and happens for clang-10 as well but not a GCC 10 prerelease build. > > > Interesting. Is that because GCC doesn't emit out-of-line GCOV functions, > > or does it emit PAC/BTI instructions for them instead? (you can disassemble > > one of the problematic opjects to have a look). > > GCC does emit some helper functions wrapping GCOV stuff but they have > appropriate annotations, eg: > > 00000000000000ac <_sub_D_00100_1>: > ac: d503245f bti c > b0: a9bf7bfd stp x29, x30, [sp, #-16]! > b4: 910003fd mov x29, sp > b8: 94000000 bl 0 <__gcov_exit> > bc: a8c17bfd ldp x29, x30, [sp], #16 > c0: d65f03c0 ret Hmm, where have the PAC/AUT instructions gone? > I can also reproduce this for clang with a trivial standalone source > file and -fprofile-arcs -mbranch-protection=bti so it's nothing funky > the kernel is doing as far as I can see. Good. > > I can't immediately see how to fix it, so your hack above might be the best > > bet for now. I'm just a little wary that it might not be limited to GCOV, > > but rather anything where the compiler provides a form of runtime. > > Indeed. I guess the nice thing with BTI is that if something goes wrong > it will do so rather visibly so unless there are situations where the > toolchain emits rarely called functions the problems will tend to be > very obvious, and it seems that clang is detecting the problem itself > and complaining loudly which makes it even more likely that if something > else is affected it'll be noticed and we can at least add similar > bodges. > > It does seem it's a straight compiler issue, if the compiler is emitting > runtime then the compiler ought to be ensuring that it agrees with the > build options the compiler was given and I can't think how this would be > fixable or avoidable outside of the compiler other than "don't do that" > which is what my Kconfig bodge did. I'm talking to the toolchain people > internally about this. Thanks. I'll apply your 'depends on ...' line locally and push that out if I don't run into any more issues. Will
On Thu, May 07, 2020 at 05:36:58PM +0100, Will Deacon wrote: > On Thu, May 07, 2020 at 05:30:45PM +0100, Mark Brown wrote: > > GCC does emit some helper functions wrapping GCOV stuff but they have > > appropriate annotations, eg: > > 00000000000000ac <_sub_D_00100_1>: > > ac: d503245f bti c > Hmm, where have the PAC/AUT instructions gone? I was testing with -mbranch-protection=bti while trying to narrow down the issue when I pasted that example in, if PAC is enabled then you get the PAC/AUT instructions instead. > > It does seem it's a straight compiler issue, if the compiler is emitting > > runtime then the compiler ought to be ensuring that it agrees with the > > build options the compiler was given and I can't think how this would be > > fixable or avoidable outside of the compiler other than "don't do that" > > which is what my Kconfig bodge did. I'm talking to the toolchain people > > internally about this. > Thanks. I'll apply your 'depends on ...' line locally and push that out > if I don't run into any more issues. Thanks, hopefully it'll be fine.
On Wed, 6 May 2020 20:51:27 +0100, Mark Brown wrote: > This patch series adds support for protecting the kernel and vDSO with > BTI including code compiled with the BPF JIT at runtime. > > We build the kernel with annotations for BTI and then map the kernel > with GP based on the support on the boot CPU, rejecting secondaries that > don't have BTI support. If there is a need to handle big.LITTLE systems > with mismatched BTI support we will have to revisit this, currently no > such implementations exist. > > [...] Applied to arm64 (for-next/bti), thanks! [01/11] arm64: Document why we enable PAC support for leaf functions https://git.kernel.org/arm64/c/717b938e22f8 [02/11] arm64: bti: Support building kernel C code using BTI https://git.kernel.org/arm64/c/92e2294d870b [03/11] arm64: asm: Override SYM_FUNC_START when building the kernel with BTI https://git.kernel.org/arm64/c/714a8d02ca4d [04/11] arm64: Set GP bit in kernel page tables to enable BTI for the kernel https://git.kernel.org/arm64/c/c8027285e366 [05/11] arm64: bpf: Annotate JITed code for BTI https://git.kernel.org/arm64/c/fa76cfe65c1d [06/11] arm64: mm: Mark executable text as guarded pages https://git.kernel.org/arm64/c/67d4a1cd0976 [07/11] arm64: bti: Provide Kconfig for kernel mode BTI https://git.kernel.org/arm64/c/97fed779f2a6 [08/11] arm64: asm: Provide a mechanism for generating ELF note for BTI https://git.kernel.org/arm64/c/3a9b136c998f [09/11] arm64: vdso: Annotate for BTI https://git.kernel.org/arm64/c/a6aadc28278a [10/11] arm64: vdso: Force the vDSO to be linked as BTI when built for BTI https://git.kernel.org/arm64/c/5e02a1887fce [11/11] arm64: vdso: Map the vDSO text with guarded pages when built for BTI https://git.kernel.org/arm64/c/bf740a905ffe Cheers,
On Thu, May 07, 2020 at 05:36:58PM +0100, Will Deacon wrote: > On Thu, May 07, 2020 at 05:30:45PM +0100, Mark Brown wrote: > > It does seem it's a straight compiler issue, if the compiler is emitting > > runtime then the compiler ought to be ensuring that it agrees with the > > build options the compiler was given and I can't think how this would be > > fixable or avoidable outside of the compiler other than "don't do that" > > which is what my Kconfig bodge did. I'm talking to the toolchain people > > internally about this. > Thanks. I'll apply your 'depends on ...' line locally and push that out > if I don't run into any more issues. Thanks. The issue should be fixed by clang upstream with: https://reviews.llvm.org/D75181 Once that is sorted out and lands I'll send followup patches opening up the dependencies to match.