| Message ID | 20200316165055.31179-1-broonie@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | arm64: Branch Target Identification support | expand |
On Mon, Mar 16, 2020 at 04:50:42PM +0000, Mark Brown wrote: > Daniel Kiss (1): > mm: smaps: Report arm64 guarded pages in smaps > > Dave Martin (11): > ELF: UAPI and Kconfig additions for ELF program properties > ELF: Add ELF program property parsing support > arm64: Basic Branch Target Identification support > elf: Allow arch to tweak initial mmap prot flags > arm64: elf: Enable BTI at exec based on ELF program properties > arm64: BTI: Decode BYTPE bits when printing PSTATE > arm64: unify native/compat instruction skipping > arm64: traps: Shuffle code to eliminate forward declarations > arm64: BTI: Reset BTYPE when skipping emulated instructions > KVM: arm64: BTI: Reset BTYPE when skipping emulated instructions > arm64: BTI: Add Kconfig entry for userspace BTI > > Mark Brown (1): > arm64: mm: Display guarded pages in ptdump I provisionally pushed this patches to linux-next (and arm64 for-next/bti). I'm not sure whether they'll make it into 5.7 yet (still missing acks on the fs/* changes) but at least they'll get some wider exposure, especially as they go outside arch/arm64/.
The 03/16/2020 16:50, Mark Brown wrote: > This patch series implements support for ARMv8.5-A Branch Target > Identification (BTI), which is a control flow integrity protection > feature introduced as part of the ARMv8.5-A extensions. i was playing with this and it seems the kernel does not add PROT_BTI to non-static executables (i.e. there is an interpreter). i thought any elf that the kernel maps would get PROT_BTI from the kernel. (i want to remove the mprotect in glibc when not necessary) i tested by linking a hello world exe with -Wl,-z,force-bti (and verified that the property note is there) and expected it to crash (with SIGILL) when the dynamic linker jumps to _start in the exe, but it executed without errors (if i do the mprotect in glibc then i get SIGILL as expected). is this deliberate? does the kernel map static exe and dynamic linked exe differently? i cant tell looking at the patches where this logic comes from. > > Changes: > > v10: > - Fix build for !COMPAT configurations. > v9: > - Move Kconfig addition to final patch in series. > - Add patch from Daniel Kiss adding BTI information to smaps, this has > a trivial conflict with a .rst conversion in -next. > v8: > - Remove a redundant IS_ENABLED(CONFIG_ARM64_BTI) check. > v7: > - Rebase onto v5.6-rc3. > - Move comment about keeping NT_GNU_PROPERTY_TYPE_0 internal into first > patch. > - Add an explicit check for system_supports_bti() when parsing BTI ELF > property for improved robustness. > v6: > - Rebase onto v5.6-rc1. > - Fix typos s/BYTPE/BTYPE/ in commit log for "arm64: BTI: Decode BYTPE > bits when printing PSTATE". > v5: > - Changed a bunch of -EIO to -ENOEXEC in the ELF parsing code. > - Move PSR_BTYPE defines to UAPI. > - Use compat_user_mode() rather than open coding. > - Fix a typo s/BYTPE/BTYPE/ in syscall.c > v4: > - Dropped patch fixing existing documentation as it has already been merged. > - Convert WARN_ON() to WARN_ON_ONCE() in "ELF: Add ELF program property > parsing support". > - Added display of guarded pages to ptdump. > - Updated for conversion of exception handling from assembler to C. > > Notes: > > * GCC 9 can compile backwards-compatible BTI-enabled code with > -mbranch-protection=bti or -mbranch-protection=standard. > > * Binutils 2.33 and later support the new ELF note. > > Creation of a BTI-enabled binary requires _everything_ linked in to > be BTI-enabled. For now ld --force-bti can be used to override this, > but some things may break until the required C library support is in > place. > > There is no straightforward way to mark a .s file as BTI-enabled: > scraping the output from gcc -S works as a quick hack for now. > > readelf -n can be used to examing the program properties in an ELF > file. > > * Runtime mmap() and mprotect() can be used to enable BTI on a > page-by-page basis using the new PROT_BTI, but the code in the > affected pages still needs to be written or compiled to contain the > appropriate BTI landing pads. > > Daniel Kiss (1): > mm: smaps: Report arm64 guarded pages in smaps > > Dave Martin (11): > ELF: UAPI and Kconfig additions for ELF program properties > ELF: Add ELF program property parsing support > arm64: Basic Branch Target Identification support > elf: Allow arch to tweak initial mmap prot flags > arm64: elf: Enable BTI at exec based on ELF program properties > arm64: BTI: Decode BYTPE bits when printing PSTATE > arm64: unify native/compat instruction skipping > arm64: traps: Shuffle code to eliminate forward declarations > arm64: BTI: Reset BTYPE when skipping emulated instructions > KVM: arm64: BTI: Reset BTYPE when skipping emulated instructions > arm64: BTI: Add Kconfig entry for userspace BTI > > Mark Brown (1): > arm64: mm: Display guarded pages in ptdump > > Documentation/arm64/cpu-feature-registers.rst | 2 + > Documentation/arm64/elf_hwcaps.rst | 5 + > Documentation/filesystems/proc.txt | 1 + > arch/arm64/Kconfig | 25 +++ > arch/arm64/include/asm/cpucaps.h | 3 +- > arch/arm64/include/asm/cpufeature.h | 6 + > arch/arm64/include/asm/elf.h | 50 ++++++ > arch/arm64/include/asm/esr.h | 2 +- > arch/arm64/include/asm/exception.h | 1 + > arch/arm64/include/asm/hwcap.h | 1 + > arch/arm64/include/asm/kvm_emulate.h | 6 +- > arch/arm64/include/asm/mman.h | 37 +++++ > arch/arm64/include/asm/pgtable-hwdef.h | 1 + > arch/arm64/include/asm/pgtable.h | 2 +- > arch/arm64/include/asm/ptrace.h | 1 + > arch/arm64/include/asm/sysreg.h | 4 + > arch/arm64/include/uapi/asm/hwcap.h | 1 + > arch/arm64/include/uapi/asm/mman.h | 9 ++ > arch/arm64/include/uapi/asm/ptrace.h | 9 ++ > arch/arm64/kernel/cpufeature.c | 33 ++++ > arch/arm64/kernel/cpuinfo.c | 1 + > arch/arm64/kernel/entry-common.c | 11 ++ > arch/arm64/kernel/process.c | 36 ++++- > arch/arm64/kernel/ptrace.c | 2 +- > arch/arm64/kernel/signal.c | 16 ++ > arch/arm64/kernel/syscall.c | 18 +++ > arch/arm64/kernel/traps.c | 131 ++++++++-------- > arch/arm64/mm/dump.c | 5 + > fs/Kconfig.binfmt | 6 + > fs/binfmt_elf.c | 145 +++++++++++++++++- > fs/compat_binfmt_elf.c | 4 + > fs/proc/task_mmu.c | 3 + > include/linux/elf.h | 43 ++++++ > include/linux/mm.h | 3 + > include/uapi/linux/elf.h | 11 ++ > 35 files changed, 560 insertions(+), 74 deletions(-) > create mode 100644 arch/arm64/include/asm/mman.h > create mode 100644 arch/arm64/include/uapi/asm/mman.h > > > base-commit: f8788d86ab28f61f7b46eb6be375f8a726783636 > -- > 2.20.1 > --
On Fri, Mar 20, 2020 at 05:39:46PM +0000, Szabolcs Nagy wrote: > The 03/16/2020 16:50, Mark Brown wrote: > > This patch series implements support for ARMv8.5-A Branch Target > > Identification (BTI), which is a control flow integrity protection > > feature introduced as part of the ARMv8.5-A extensions. > > i was playing with this and it seems the kernel does not add > PROT_BTI to non-static executables (i.e. there is an interpreter). > > i thought any elf that the kernel maps would get PROT_BTI from the > kernel. (i want to remove the mprotect in glibc when not necessary) I haven't followed the early discussions but I think this makes sense. > i tested by linking a hello world exe with -Wl,-z,force-bti (and > verified that the property note is there) and expected it to crash > (with SIGILL) when the dynamic linker jumps to _start in the exe, > but it executed without errors (if i do the mprotect in glibc then > i get SIGILL as expected). > > is this deliberate? does the kernel map static exe and dynamic > linked exe differently? I think the logic is in patch 5: +int arch_elf_adjust_prot(int prot, const struct arch_elf_state *state, + bool has_interp, bool is_interp) +{ + if (is_interp != has_interp) + return prot; + + if (!(state->flags & ARM64_ELF_BTI)) + return prot; + + if (prot & PROT_EXEC) + prot |= PROT_BTI; + + return prot; +} At a quick look, for dynamic binaries we have has_interp == true and is_interp == false. I don't know why but, either way, the above code needs a comment with some justification.
On Mon, Mar 23, 2020 at 12:21:44PM +0000, Catalin Marinas wrote: > On Fri, Mar 20, 2020 at 05:39:46PM +0000, Szabolcs Nagy wrote: > +int arch_elf_adjust_prot(int prot, const struct arch_elf_state *state, > + bool has_interp, bool is_interp) > +{ > + if (is_interp != has_interp) > + return prot; > + > + if (!(state->flags & ARM64_ELF_BTI)) > + return prot; > + > + if (prot & PROT_EXEC) > + prot |= PROT_BTI; > + > + return prot; > +} > At a quick look, for dynamic binaries we have has_interp == true and > is_interp == false. I don't know why but, either way, the above code > needs a comment with some justification. I don't really know for certain either, I inherited this code as is with the understanding that this was all agreed with the toolchain and libc people - the actual discussion that lead to the decisions being made happened before I was involved. My understanding is that the idea was that the dynamic linker would be responsible for mapping everything in dynamic applications other than itself but other than consistency I don't know why. I guess it defers more decision making to userspace but I'm having a hard time thinking of sensible cases where one might wish to make a decision other than enabling PROT_BTI. I'd be perfectly happy to drop the check if that makes more sense to people, otherwise I can send a patch adding a comment explaining the situation.
On Mon, Mar 23, 2020 at 01:24:12PM +0000, Mark Brown wrote: > On Mon, Mar 23, 2020 at 12:21:44PM +0000, Catalin Marinas wrote: > > On Fri, Mar 20, 2020 at 05:39:46PM +0000, Szabolcs Nagy wrote: > > > +int arch_elf_adjust_prot(int prot, const struct arch_elf_state *state, > > + bool has_interp, bool is_interp) > > +{ > > + if (is_interp != has_interp) > > + return prot; > > + > > + if (!(state->flags & ARM64_ELF_BTI)) > > + return prot; > > + > > + if (prot & PROT_EXEC) > > + prot |= PROT_BTI; > > + > > + return prot; > > +} > > > At a quick look, for dynamic binaries we have has_interp == true and > > is_interp == false. I don't know why but, either way, the above code > > needs a comment with some justification. > > I don't really know for certain either, I inherited this code as is with > the understanding that this was all agreed with the toolchain and libc > people - the actual discussion that lead to the decisions being made > happened before I was involved. My understanding is that the idea was > that the dynamic linker would be responsible for mapping everything in > dynamic applications other than itself but other than consistency I > don't know why. I guess it defers more decision making to userspace but > I'm having a hard time thinking of sensible cases where one might wish > to make a decision other than enabling PROT_BTI. My understanding was this had been agreed with the toolchain folk a while back -- anything static loaded by the kernel (i.e. a static executable or the dynamic linker) would get GP set. In other cases the linker will mess with the permissions on the pages anyhow, and needs to be aware of BTI in order to do the right thing, so it was better to leave it to userspace consistently (e.g. as that had the least risk of subtle changes in behaviour leading to ABI difficulties). > I'd be perfectly happy to drop the check if that makes more sense to > people, otherwise I can send a patch adding a comment explaining the > situation. I think it would be best to document the current behaviour, as it's a simple ABI that we can guarantee, and the dynamic linker will have to be aware of BTI in order to do the right thing anyhow. Thanks, Mark.
On Mon, Mar 23, 2020 at 01:57:22PM +0000, Mark Rutland wrote: > On Mon, Mar 23, 2020 at 01:24:12PM +0000, Mark Brown wrote: > > On Mon, Mar 23, 2020 at 12:21:44PM +0000, Catalin Marinas wrote: > > > On Fri, Mar 20, 2020 at 05:39:46PM +0000, Szabolcs Nagy wrote: > > > > > +int arch_elf_adjust_prot(int prot, const struct arch_elf_state *state, > > > + bool has_interp, bool is_interp) > > > +{ > > > + if (is_interp != has_interp) > > > + return prot; > > > + > > > + if (!(state->flags & ARM64_ELF_BTI)) > > > + return prot; > > > + > > > + if (prot & PROT_EXEC) > > > + prot |= PROT_BTI; > > > + > > > + return prot; > > > +} > > > > > At a quick look, for dynamic binaries we have has_interp == true and > > > is_interp == false. I don't know why but, either way, the above code > > > needs a comment with some justification. > > > > I don't really know for certain either, I inherited this code as is with > > the understanding that this was all agreed with the toolchain and libc > > people - the actual discussion that lead to the decisions being made > > happened before I was involved. My understanding is that the idea was > > that the dynamic linker would be responsible for mapping everything in > > dynamic applications other than itself but other than consistency I > > don't know why. I guess it defers more decision making to userspace but > > I'm having a hard time thinking of sensible cases where one might wish > > to make a decision other than enabling PROT_BTI. > > My understanding was this had been agreed with the toolchain folk a > while back -- anything static loaded by the kernel (i.e. a static > executable or the dynamic linker) would get GP set. In other cases the > linker will mess with the permissions on the pages anyhow, and needs to > be aware of BTI in order to do the right thing, so it was better to > leave it to userspace consistently (e.g. as that had the least risk of > subtle changes in behaviour leading to ABI difficulties). So this means that the interpreter will have to mprotect(PROT_BTI) the text section of the primary executable. For subsequent libraries, it calls mmap() explicitly anyway but not for the main executable (IIUC). > > I'd be perfectly happy to drop the check if that makes more sense to > > people, otherwise I can send a patch adding a comment explaining the > > situation. > > I think it would be best to document the current behaviour, as it's a > simple ABI that we can guarantee, and the dynamic linker will have to be > aware of BTI in order to do the right thing anyhow. That's a valid point. If we have an old dynamic linker and the kernel enabled BTI automatically for the main executable, could things go wrong (e.g. does the PLT need to be BTI-aware)?
On Mon, Mar 23, 2020 at 02:39:55PM +0000, Catalin Marinas wrote: > On Mon, Mar 23, 2020 at 01:57:22PM +0000, Mark Rutland wrote: > > On Mon, Mar 23, 2020 at 01:24:12PM +0000, Mark Brown wrote: > > > On Mon, Mar 23, 2020 at 12:21:44PM +0000, Catalin Marinas wrote: > > > > On Fri, Mar 20, 2020 at 05:39:46PM +0000, Szabolcs Nagy wrote: > > > > > > > +int arch_elf_adjust_prot(int prot, const struct arch_elf_state *state, > > > > + bool has_interp, bool is_interp) > > > > +{ > > > > + if (is_interp != has_interp) > > > > + return prot; > > > > + > > > > + if (!(state->flags & ARM64_ELF_BTI)) > > > > + return prot; > > > > + > > > > + if (prot & PROT_EXEC) > > > > + prot |= PROT_BTI; > > > > + > > > > + return prot; > > > > +} > > > > > > > At a quick look, for dynamic binaries we have has_interp == true and > > > > is_interp == false. I don't know why but, either way, the above code > > > > needs a comment with some justification. > > > > > > I don't really know for certain either, I inherited this code as is with > > > the understanding that this was all agreed with the toolchain and libc > > > people - the actual discussion that lead to the decisions being made > > > happened before I was involved. My understanding is that the idea was > > > that the dynamic linker would be responsible for mapping everything in > > > dynamic applications other than itself but other than consistency I > > > don't know why. I guess it defers more decision making to userspace but > > > I'm having a hard time thinking of sensible cases where one might wish > > > to make a decision other than enabling PROT_BTI. > > > > My understanding was this had been agreed with the toolchain folk a > > while back -- anything static loaded by the kernel (i.e. a static > > executable or the dynamic linker) would get GP set. In other cases the > > linker will mess with the permissions on the pages anyhow, and needs to > > be aware of BTI in order to do the right thing, so it was better to > > leave it to userspace consistently (e.g. as that had the least risk of > > subtle changes in behaviour leading to ABI difficulties). > > So this means that the interpreter will have to mprotect(PROT_BTI) the > text section of the primary executable. Yes, but after fixing up any relocations in that section it's going to have to call mprotect() on it anyhow (e.g. in order to make it read-only), and in doing so would throw away BTI unless it was BTI aware. > > I think it would be best to document the current behaviour, as it's a > > simple ABI that we can guarantee, and the dynamic linker will have to be > > aware of BTI in order to do the right thing anyhow. > > That's a valid point. If we have an old dynamic linker and the kernel > enabled BTI automatically for the main executable, could things go wrong > (e.g. does the PLT need to be BTI-aware)? I believe that a PLT in an unguarded page needs no special treatment. A PLT within a guarded page needs to be built specially for BTI. Thanks, Mark.
On Mon, Mar 23, 2020 at 02:39:55PM +0000, Catalin Marinas wrote: > On Mon, Mar 23, 2020 at 01:57:22PM +0000, Mark Rutland wrote: > > On Mon, Mar 23, 2020 at 01:24:12PM +0000, Mark Brown wrote: > > > On Mon, Mar 23, 2020 at 12:21:44PM +0000, Catalin Marinas wrote: > > > > On Fri, Mar 20, 2020 at 05:39:46PM +0000, Szabolcs Nagy wrote: > > > > > > > +int arch_elf_adjust_prot(int prot, const struct arch_elf_state *state, > > > > + bool has_interp, bool is_interp) > > > > +{ > > > > + if (is_interp != has_interp) > > > > + return prot; > > > > + > > > > + if (!(state->flags & ARM64_ELF_BTI)) > > > > + return prot; > > > > + > > > > + if (prot & PROT_EXEC) > > > > + prot |= PROT_BTI; > > > > + > > > > + return prot; > > > > +} > > I think it would be best to document the current behaviour, as it's a > > simple ABI that we can guarantee, and the dynamic linker will have to be > > aware of BTI in order to do the right thing anyhow. > > That's a valid point. If we have an old dynamic linker and the kernel > enabled BTI automatically for the main executable, could things go wrong > (e.g. does the PLT need to be BTI-aware)? Also worth noting that an old dynamic linker won't have ARM64_ELF_BTI set, so the kernel will not enable BTI for this. Mark.
On Mon, Mar 23, 2020 at 02:55:46PM +0000, Mark Rutland wrote: > On Mon, Mar 23, 2020 at 02:39:55PM +0000, Catalin Marinas wrote: > > So this means that the interpreter will have to mprotect(PROT_BTI) the > > text section of the primary executable. > Yes, but after fixing up any relocations in that section it's going to > have to call mprotect() on it anyhow (e.g. in order to make it > read-only), and in doing so would throw away BTI unless it was BTI > aware. Ah, of course - I forgot that's not a read/modify/write cycle. I'll send the comment version. > > That's a valid point. If we have an old dynamic linker and the kernel > > enabled BTI automatically for the main executable, could things go wrong > > (e.g. does the PLT need to be BTI-aware)? > I believe that a PLT in an unguarded page needs no special treatment. A > PLT within a guarded page needs to be built specially for BTI. Unguarded stuff is unaffected.
The 03/23/2020 14:55, Mark Rutland wrote: > On Mon, Mar 23, 2020 at 02:39:55PM +0000, Catalin Marinas wrote: > > On Mon, Mar 23, 2020 at 01:57:22PM +0000, Mark Rutland wrote: > > > On Mon, Mar 23, 2020 at 01:24:12PM +0000, Mark Brown wrote: > > > > On Mon, Mar 23, 2020 at 12:21:44PM +0000, Catalin Marinas wrote: > > > > > On Fri, Mar 20, 2020 at 05:39:46PM +0000, Szabolcs Nagy wrote: > > > > > > > > > +int arch_elf_adjust_prot(int prot, const struct arch_elf_state *state, > > > > > + bool has_interp, bool is_interp) > > > > > +{ > > > > > + if (is_interp != has_interp) > > > > > + return prot; > > > > > + > > > > > + if (!(state->flags & ARM64_ELF_BTI)) > > > > > + return prot; > > > > > + > > > > > + if (prot & PROT_EXEC) > > > > > + prot |= PROT_BTI; > > > > > + > > > > > + return prot; > > > > > +} > > > > > > > > > At a quick look, for dynamic binaries we have has_interp == true and > > > > > is_interp == false. I don't know why but, either way, the above code > > > > > needs a comment with some justification. > > > > > > > > I don't really know for certain either, I inherited this code as is with > > > > the understanding that this was all agreed with the toolchain and libc > > > > people - the actual discussion that lead to the decisions being made > > > > happened before I was involved. My understanding is that the idea was > > > > that the dynamic linker would be responsible for mapping everything in > > > > dynamic applications other than itself but other than consistency I > > > > don't know why. I guess it defers more decision making to userspace but > > > > I'm having a hard time thinking of sensible cases where one might wish > > > > to make a decision other than enabling PROT_BTI. > > > > > > My understanding was this had been agreed with the toolchain folk a > > > while back -- anything static loaded by the kernel (i.e. a static > > > executable or the dynamic linker) would get GP set. In other cases the > > > linker will mess with the permissions on the pages anyhow, and needs to > > > be aware of BTI in order to do the right thing, so it was better to > > > leave it to userspace consistently (e.g. as that had the least risk of > > > subtle changes in behaviour leading to ABI difficulties). > > > > So this means that the interpreter will have to mprotect(PROT_BTI) the > > text section of the primary executable. > > Yes, but after fixing up any relocations in that section it's going to > have to call mprotect() on it anyhow (e.g. in order to make it > read-only), and in doing so would throw away BTI unless it was BTI > aware. note: on the main exe only one mprotect is used in case there is PT_GNU_RELRO (or DF_BIND_NOW) to mark part of the rw data segment read only. so if PROT_BTI on main exe is ld.so responsibility that adds one more syscall to the program startup (not a huge cost). (currently executable segment can be mprotected by libc in case of text relocations but those are not fully supported and won't work under various kernel hardening schemes that disallow exec+write mappings)
On Mon, Mar 16, 2020 at 04:50:42PM +0000, Mark Brown wrote: > This patch series implements support for ARMv8.5-A Branch Target > Identification (BTI), which is a control flow integrity protection > feature introduced as part of the ARMv8.5-A extensions. I've not resent this since the branch is still sitting in the arm64 tree but it's also not in -next at the minute - is there anything you're waiting for from my end here?
On Wed, Apr 22, 2020 at 04:44:36PM +0100, Mark Brown wrote: > On Mon, Mar 16, 2020 at 04:50:42PM +0000, Mark Brown wrote: > > This patch series implements support for ARMv8.5-A Branch Target > > Identification (BTI), which is a control flow integrity protection > > feature introduced as part of the ARMv8.5-A extensions. > > I've not resent this since the branch is still sitting in the arm64 tree > but it's also not in -next at the minute - is there anything you're > waiting for from my end here? It's up to Will whether he wants a new series posted. The for-next/bti branch is complete AFAICT, only that normally we start queueing stuff (and push to -next) around -rc3.
On Wed, Apr 22, 2020 at 05:29:54PM +0100, Catalin Marinas wrote: > On Wed, Apr 22, 2020 at 04:44:36PM +0100, Mark Brown wrote: > > On Mon, Mar 16, 2020 at 04:50:42PM +0000, Mark Brown wrote: > > > This patch series implements support for ARMv8.5-A Branch Target > > > Identification (BTI), which is a control flow integrity protection > > > feature introduced as part of the ARMv8.5-A extensions. > > > > I've not resent this since the branch is still sitting in the arm64 tree > > but it's also not in -next at the minute - is there anything you're > > waiting for from my end here? > > It's up to Will whether he wants a new series posted. The for-next/bti > branch is complete AFAICT, only that normally we start queueing stuff > (and push to -next) around -rc3. I'm happy either way, but it would be nice to base other BTI patches on top of this branch. Mark -- is it easier for you to refresh the series against v5.7-rc3, or leave it like it is? Please just let me know either way. Thanks, Will
On Tue, Apr 28, 2020 at 02:28:05PM +0100, Will Deacon wrote: > I'm happy either way, but it would be nice to base other BTI patches on > top of this branch. Mark -- is it easier for you to refresh the series > against v5.7-rc3, or leave it like it is? Please just let me know either > way. It's probably easier for me if you just use the existing branch, I've already got a branch based on a merge down.
On Tue, Apr 28, 2020 at 04:12:05PM +0100, Mark Brown wrote: > On Tue, Apr 28, 2020 at 02:28:05PM +0100, Will Deacon wrote: > > > I'm happy either way, but it would be nice to base other BTI patches on > > top of this branch. Mark -- is it easier for you to refresh the series > > against v5.7-rc3, or leave it like it is? Please just let me know either > > way. > > It's probably easier for me if you just use the existing branch, I've > already got a branch based on a merge down. Okey doke, I'll funnel that in the direction of linux-next then. It does mean that any subsequent patches for 5.8 that depend on BTI will need to be based on this branch, so as long as you're ok with that then it's fine by me (since I won't be able to apply patches if they refer to changes introduced in the recent merge window). Will
On Tue, Apr 28, 2020 at 04:18:16PM +0100, Will Deacon wrote: > On Tue, Apr 28, 2020 at 04:12:05PM +0100, Mark Brown wrote: > > It's probably easier for me if you just use the existing branch, I've > > already got a branch based on a merge down. > Okey doke, I'll funnel that in the direction of linux-next then. It does > mean that any subsequent patches for 5.8 that depend on BTI will need to > be based on this branch, so as long as you're ok with that then it's fine > by me (since I won't be able to apply patches if they refer to changes > introduced in the recent merge window). That's not a problem, that's what I've got already and if I try to send everything based off -rc3 directly the series would get unmanagably large. Actually unless you think it's a bad idea I think what I'll do is go and send out a couple of the preparatory changes (the insn updates and the last bit of annotation conversions) separately for that branch while I finalize the revisions of the main BTI kernel bit, hopefully that'll make the review a bit more approachable.
On Tue, Apr 28, 2020 at 04:58:12PM +0100, Mark Brown wrote: > On Tue, Apr 28, 2020 at 04:18:16PM +0100, Will Deacon wrote: > > On Tue, Apr 28, 2020 at 04:12:05PM +0100, Mark Brown wrote: > > > > It's probably easier for me if you just use the existing branch, I've > > > already got a branch based on a merge down. > > > Okey doke, I'll funnel that in the direction of linux-next then. It does > > mean that any subsequent patches for 5.8 that depend on BTI will need to > > be based on this branch, so as long as you're ok with that then it's fine > > by me (since I won't be able to apply patches if they refer to changes > > introduced in the recent merge window). > > That's not a problem, that's what I've got already and if I try to send > everything based off -rc3 directly the series would get unmanagably > large. Actually unless you think it's a bad idea I think what I'll do > is go and send out a couple of the preparatory changes (the insn updates > and the last bit of annotation conversions) separately for that branch > while I finalize the revisions of the main BTI kernel bit, hopefully > that'll make the review a bit more approachable. Okey doke, sounds good to me. I'm queuing stuff atm, so as long you tell me what I need to apply things against then we should be good. Will
On Tue, Apr 28, 2020 at 05:01:43PM +0100, Will Deacon wrote: > On Tue, Apr 28, 2020 at 04:58:12PM +0100, Mark Brown wrote: > > On Tue, Apr 28, 2020 at 04:18:16PM +0100, Will Deacon wrote: > > > On Tue, Apr 28, 2020 at 04:12:05PM +0100, Mark Brown wrote: > > > > > > It's probably easier for me if you just use the existing branch, I've > > > > already got a branch based on a merge down. > > > > > Okey doke, I'll funnel that in the direction of linux-next then. It does > > > mean that any subsequent patches for 5.8 that depend on BTI will need to > > > be based on this branch, so as long as you're ok with that then it's fine > > > by me (since I won't be able to apply patches if they refer to changes > > > introduced in the recent merge window). > > > > That's not a problem, that's what I've got already and if I try to send > > everything based off -rc3 directly the series would get unmanagably > > large. Actually unless you think it's a bad idea I think what I'll do > > is go and send out a couple of the preparatory changes (the insn updates > > and the last bit of annotation conversions) separately for that branch > > while I finalize the revisions of the main BTI kernel bit, hopefully > > that'll make the review a bit more approachable. > > Okey doke, sounds good to me. I'm queuing stuff atm, so as long you tell > me what I need to apply things against then we should be good. Just a heads up: I've renamed for-next/bti to for-next/bti-user, so it doesn't get confusing with the pending in-kernel BTI patches. All the commit SHAs remain unchanged. Will