| Message ID | 20200227174417.23722-1-broonie@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | arm64: Branch Target Identification support | expand |
On 2/27/20 9:44 AM, Mark Brown wrote: > * Binutils trunk supports the new ELF note, but this wasn't in a release > the last time I posted this series. (The situation _might_ have changed > in the meantime...) I believe this support is in binutils 2.32. r~
On Thu, Feb 27, 2020 at 05:44:06PM +0000, Mark Brown wrote: > Dave Martin (10): > 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 Al, are you ok for patches 1, 2 and 4 in this series to be merged via the arm64 tree? The full series is here: https://lore.kernel.org/linux-arm-kernel/20200227174417.23722-1-broonie@kernel.org/ Thanks.
On Thu, Feb 27, 2020 at 05:44:06PM +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. Does this series affect uprobes in any way? I.e. can you probe a landing pad?
On Fri, Mar 06, 2020 at 10:27:29AM +0000, Catalin Marinas wrote: > On Thu, Feb 27, 2020 at 05:44:06PM +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. > Does this series affect uprobes in any way? I.e. can you probe a landing > pad? You can't probe a landing pad, uprobes on landing pads will be silently ignored so the program isn't disrupted, you just don't get the expected trace from those uprobes. This isn't new with the BTI support since the landing pads are generally pointer auth instructions, these already can't be probed regardless of what's going on with this series. It's already on the list to get sorted.
On Mon, Mar 09, 2020 at 09:05:05PM +0000, Mark Brown wrote: > On Fri, Mar 06, 2020 at 10:27:29AM +0000, Catalin Marinas wrote: > > Does this series affect uprobes in any way? I.e. can you probe a landing > > pad? > You can't probe a landing pad, uprobes on landing pads will be silently > ignored so the program isn't disrupted, you just don't get the expected > trace from those uprobes. This isn't new with the BTI support since > the landing pads are generally pointer auth instructions, these already > can't be probed regardless of what's going on with this series. It's > already on the list to get sorted. Sorry, I realized thanks to Amit's off-list prompting that I was testing that I was verifying with the wrong kernel binary here (user error since it took me a while to sort out uprobes) so this isn't quite right - you can probe the landing pads with or without this series.
On Tue, Mar 10, 2020 at 12:42:26PM +0000, Mark Brown wrote: > On Mon, Mar 09, 2020 at 09:05:05PM +0000, Mark Brown wrote: > > On Fri, Mar 06, 2020 at 10:27:29AM +0000, Catalin Marinas wrote: > > > > Does this series affect uprobes in any way? I.e. can you probe a landing > > > pad? > > > You can't probe a landing pad, uprobes on landing pads will be silently > > ignored so the program isn't disrupted, you just don't get the expected > > trace from those uprobes. This isn't new with the BTI support since > > the landing pads are generally pointer auth instructions, these already > > can't be probed regardless of what's going on with this series. It's > > already on the list to get sorted. > > Sorry, I realized thanks to Amit's off-list prompting that I was testing > that I was verifying with the wrong kernel binary here (user error since > it took me a while to sort out uprobes) so this isn't quite right - you > can probe the landing pads with or without this series. Can we not change aarch64_insn_is_nop() to actually return true only for NOP and ignore everything else in the hint space? We tend to re-use the hint instructions for new things in the architecture, so I'd rather white-list what we know we can safely probe than black-listing only some of the hint instructions. I haven't assessed the effort of doing the above (probably not a lot) but as a short-term workaround we could add the BTI and PAC hint instructions to the aarch64_insn_is_nop() (though my preferred option is the white-list one).
On Wed, Mar 11, 2020 at 04:28:58PM +0000, Catalin Marinas wrote: > On Tue, Mar 10, 2020 at 12:42:26PM +0000, Mark Brown wrote: > > Sorry, I realized thanks to Amit's off-list prompting that I was testing > > that I was verifying with the wrong kernel binary here (user error since > > it took me a while to sort out uprobes) so this isn't quite right - you > > can probe the landing pads with or without this series. > Can we not change aarch64_insn_is_nop() to actually return true only for > NOP and ignore everything else in the hint space? We tend to re-use the > hint instructions for new things in the architecture, so I'd rather > white-list what we know we can safely probe than black-listing only some > of the hint instructions. That's literally the patch I am sitting on which made the difference with the testing on the wrong binary. > I haven't assessed the effort of doing the above (probably not a lot) > but as a short-term workaround we could add the BTI and PAC hint > instructions to the aarch64_insn_is_nop() (though my preferred option is > the white-list one). The only thing I've seen in testing with just NOPs whitelisted is an inability to probe the PAC instructions which isn't the best user experience, especially since the effect is that the probes get silently ignored. This isn't extensive userspace testing though. Adding whitelisting of the BTI and PAC hints would definitely be a safer as a first step though. I can post either version?
On Thu, Feb 27, 2020 at 05:35:39PM -0800, Richard Henderson wrote: > On 2/27/20 9:44 AM, Mark Brown wrote: > > * Binutils trunk supports the new ELF note, but this wasn't in a release > > the last time I posted this series. (The situation _might_ have changed > > in the meantime...) > I believe this support is in binutils 2.32. It looks like it's actually 2.33 but either way it's in a release, thanks for prompting me to check. I've updated this for v9.
On Wed, Mar 11, 2020 at 05:25:56PM +0000, Mark Brown wrote: > On Wed, Mar 11, 2020 at 04:28:58PM +0000, Catalin Marinas wrote: > > On Tue, Mar 10, 2020 at 12:42:26PM +0000, Mark Brown wrote: > > > Sorry, I realized thanks to Amit's off-list prompting that I was testing > > > that I was verifying with the wrong kernel binary here (user error since > > > it took me a while to sort out uprobes) so this isn't quite right - you > > > can probe the landing pads with or without this series. > > > Can we not change aarch64_insn_is_nop() to actually return true only for > > NOP and ignore everything else in the hint space? We tend to re-use the > > hint instructions for new things in the architecture, so I'd rather > > white-list what we know we can safely probe than black-listing only some > > of the hint instructions. [...] > > I haven't assessed the effort of doing the above (probably not a lot) > > but as a short-term workaround we could add the BTI and PAC hint > > instructions to the aarch64_insn_is_nop() (though my preferred option is > > the white-list one). > > The only thing I've seen in testing with just NOPs whitelisted is an > inability to probe the PAC instructions which isn't the best user > experience, especially since the effect is that the probes get silently > ignored. This isn't extensive userspace testing though. Adding > whitelisting of the BTI and PAC hints would definitely be a safer as a > first step though. I can post either version? I thought BTI and PAC are already whitelisted in mainline as they fall into the hint space (by whitelisting I mean you can probe them). I'm trying to understand how the BTI patches affect the current uprobes support and what is needed. Executing BTI or PCI?SP out of line should be fine as they don't generate a BTI exception (the BRK doesn't either, just the normal debug exception). I think (it needs checking) that BRK preserves the PSTATE.BTYPE in SPSR. If we probe an instruction in a guarded page and then we single-step it in a non-guarded page, we'll miss a potential BTI fault. Is this an issue? If we are to keep the BTI faulting behaviour, we'd need an additional xol page, guarded, and to find a way to report the original probed address of the fault rather than the xol page. So, IIUC, we don't have an issue with the actual BTI or PACI?SP instructions but rather the other instructions that would not fault with the BTI support. While we should try to address this, I think the important bit now is not to break the existing uprobes support when running a binary with BTI enabled. Have I missed anything?
On Thu, Mar 12, 2020 at 06:42:11PM +0000, Catalin Marinas wrote: > On Wed, Mar 11, 2020 at 05:25:56PM +0000, Mark Brown wrote: > > On Wed, Mar 11, 2020 at 04:28:58PM +0000, Catalin Marinas wrote: > > > Can we not change aarch64_insn_is_nop() to actually return true only for > > > NOP and ignore everything else in the hint space? We tend to re-use the > > ignored. This isn't extensive userspace testing though. Adding > > whitelisting of the BTI and PAC hints would definitely be a safer as a > > first step though. I can post either version? > I thought BTI and PAC are already whitelisted in mainline as they fall > into the hint space (by whitelisting I mean you can probe them). This was in the context of your comment above about modifying aarch64_insn_is_nop() - if we do that and nothing else then we'd remove the current whitelisting. > I'm trying to understand how the BTI patches affect the current uprobes > support and what is needed. Executing BTI or PCI?SP out of line should > be fine as they don't generate a BTI exception (the BRK doesn't either, > just the normal debug exception). Right. > I think (it needs checking) that BRK preserves the PSTATE.BTYPE in SPSR. Yes, Exception_SoftwareBreakpoint preserves PSTATE.BTYPE. > If we probe an instruction in a guarded page and then we single-step it > in a non-guarded page, we'll miss a potential BTI fault. Is this an > issue? Obviously the main thing here is that if we miss faults then that's potentially opening something that could be used as part of an exploit chain. I'm not aware of any sensible applications that would generate the exceptions in normal operation. > If we are to keep the BTI faulting behaviour, we'd need an additional > xol page, guarded, and to find a way to report the original probed > address of the fault rather than the xol page. Yes, or just accept the inaccurate fault address which isn't good but might be the least worst thing if there's issues with reporting the original address. > So, IIUC, we don't have an issue with the actual BTI or PACI?SP > instructions but rather the other instructions that would not fault with > the BTI support. While we should try to address this, I think the > important bit now is not to break the existing uprobes support when > running a binary with BTI enabled. I think so, and as far as my ability to tell goes the worst consequence would be missing exceptions like you say. That's not great but it's at least an extra hoop people have to jump through.