mbox

[0/4] Add AARCH64 pointer authentication support

Message ID 20220425140311.95231-1-german.gomez@arm.com
Headers

Message

German Gomez April 25, 2022, 2:03 p.m. UTC
  Hi,

I've included a set of patches in order to demangle return addresses in
aarch64 platforms with pointer authentication.

Besides adding the implementation of the negate_ra_state opcode, there
is a new function in the libdwfl.h header to feed the PAC masks to the
library.

Let me know if there are any concerns with the current version.

Thanks,
German

German Gomez (4):
  aarch64: Create definitions for AARCH64_RA_SIGN_STATE register
  libdw,aarch64: Implement DW_CFA_AARCH64_negate_ra_state CFI
    instruction
  libdwfl,aarch64: Demangle return addresses using a PAC mask
  libdwfl,eu-stack,aarch64: Add API for setting AARCH64 PAC mask.

 backends/aarch64_init.c    |  6 +++---
 backends/aarch64_initreg.c |  2 ++
 backends/aarch64_regs.c    |  5 ++++-
 libdw/cfi.c                | 14 +++++++++++++-
 libdw/dwarf.h              |  5 +++++
 libdw/libdw.map            |  5 +++++
 libdwfl/dwfl_frame.c       |  3 +++
 libdwfl/dwfl_frame_regs.c  | 10 ++++++++++
 libdwfl/frame_unwind.c     | 14 +++++++++++++-
 libdwfl/libdwfl.h          |  6 ++++++
 libdwfl/libdwflP.h         |  7 +++++++
 libdwfl/linux-pid-attach.c | 34 ++++++++++++++++++++++++++++++++--
 tests/run-addrcfi.sh       |  1 +
 tests/run-allregs.sh       |  1 +
 14 files changed, 105 insertions(+), 8 deletions(-)
  

Comments

Mark Wielaard Feb. 24, 2023, 12:17 a.m. UTC | #1
Hi German,

On Thu, May 19, 2022 at 02:30:23PM +0100, German Gomez via Elfutils-devel wrote:
> On 28/04/2022 20:56, Mark Wielaard wrote:
> > I haven't been able to look at the actual patches yet. And I am on
> > vacation this week. But I'll review next week after I am back.
> 
> Thanks a lot for looking.

And then it took a couple of months to actually review the patches,
sorry.

The first two patches look OK with one small issue where/how to
define DW_AARCH64_RA_SIGN_STATE (lets put it in cfi.h instead of
dwarf.h).

I have some concerns about the last two patches because they define
new public api that is aarch64 specific.

> > A quick scan shows we need a aarch64 special public function, which
> > would be slightly ugly imho. I had hoped it could be a variant of the
> > func_addr_mask. But maybe this is too different to make more generic.
> 
> I did consider func_addr_mask initially, but when I wrote the patch it
> wasn't exposed as a perf-thread value. Currently PAC masks are constant
> but might be different from thread to thread in the future. So I placed
> it in the Thread struct.

Aha. I assumed it was per process, but it makes sense if it is
per-thread.

> I agree the arch-specific naming is not pretty. I think I can certainly
> rework it into a more generic feature. But I think I would need to make
> sure that the masks can be supplied to the Thread struct before the   
> unwind.

Are there any other architectures with a similar "mask"?

Maybe we can make it a special dwfl_thread_state_registers call with
e.g. firstreg -1 and nregs 1? That way we don't need a new function,
just a "magic" negative register number.

Also we need to extract the pauth mask somehow in
libdwfl/linux-core-attach.c

Cheers,

Mark