Re: [PATCH 0/4] Add AARCH64 pointer authentication support

[Mark Wielaard <mark@klomp.org>]

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