Re: git: 6713be315900 - main - Add NT_ARM_ADDR_MASK

On 22 Feb 2022, at 17:27, Andrew Turner <andrew@FreeBSD.org> wrote:
> 
> The branch main has been updated by andrew:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=6713be3159000783f7aacf3ea90d6c6878c44da0
> 
> commit 6713be3159000783f7aacf3ea90d6c6878c44da0
> Author:     Andrew Turner <andrew@FreeBSD.org>
> AuthorDate: 2021-08-30 16:43:22 +0000
> Commit:     Andrew Turner <andrew@FreeBSD.org>
> CommitDate: 2022-02-22 17:10:35 +0000
> 
>    Add NT_ARM_ADDR_MASK
> 
>    This can be used by debuggers to find which bits in a virtual address
>    should be masked off to get a canonical address. This is currently used
>    by the Pointer Authentication Code support to get its mask. It could also
>    be used if we support Top Byte Ignore for the same purpose.
> 
>    Reviewed by:    kib
>    Sponsored by:   The FreeBSD Foundation
>    Differential Revision: https://reviews.freebsd.org/D34302

This definition seems backwards? In Linux it’s NT_ARM_PAC_MASK, i.e.
the mask for the PAC bits, but you’ve called it NT_ARM_ADDR_MASK here,
which sounds like the mask you apply to get the address bits, but it’s
not, you apply the negation of this mask to get the address bits.

Jess

> ---
> sys/arm64/arm64/elf_machdep.c | 42 ++++++++++++++++++++++++++++++++++++++++--
> sys/arm64/arm64/ptrauth.c     |  8 +++++++-
> sys/arm64/include/cpu.h       |  3 +++
> sys/arm64/include/reg.h       |  5 +++++
> sys/arm64/include/vmparam.h   |  3 +++
> sys/sys/elf_common.h          |  1 +
> tests/sys/kern/ptrace_test.c  | 13 +++++++++++++
> 7 files changed, 72 insertions(+), 3 deletions(-)
> 
> diff --git a/sys/arm64/arm64/elf_machdep.c b/sys/arm64/arm64/elf_machdep.c
> index d98512b71e6c..383a0911b7fe 100644
> --- a/sys/arm64/arm64/elf_machdep.c
> +++ b/sys/arm64/arm64/elf_machdep.c
> @@ -59,6 +59,8 @@ __FBSDID("$FreeBSD$");
> u_long __read_frequently elf_hwcap;
> u_long __read_frequently elf_hwcap2;
> 
> +struct arm64_addr_mask elf64_addr_mask;
> +
> static struct sysentvec elf64_freebsd_sysvec = {
> 	.sv_size	= SYS_MAXSYSCALL,
> 	.sv_table	= sysent,
> @@ -119,11 +121,47 @@ static Elf64_Brandinfo freebsd_brand_info = {
> SYSINIT(elf64, SI_SUB_EXEC, SI_ORDER_FIRST,
>     (sysinit_cfunc_t)elf64_insert_brand_entry, &freebsd_brand_info);
> 
> +static bool
> +get_arm64_addr_mask(struct regset *rs, struct thread *td, void *buf,
> +    size_t *sizep)
> +{
> +	if (buf != NULL) {
> +		KASSERT(*sizep == sizeof(elf64_addr_mask),
> +		    ("%s: invalid size", __func__));
> +		memcpy(buf, &elf64_addr_mask, sizeof(elf64_addr_mask));
> +	}
> +	*sizep = sizeof(elf64_addr_mask);
> +
> +	return (true);
> +}
> +
> +static struct regset regset_arm64_addr_mask = {
> +	.note = NT_ARM_ADDR_MASK,
> +	.size = sizeof(struct arm64_addr_mask),
> +	.get = get_arm64_addr_mask,
> +};
> +ELF_REGSET(regset_arm64_addr_mask);
> +
> void
> -elf64_dump_thread(struct thread *td __unused, void *dst __unused,
> -    size_t *off __unused)
> +elf64_dump_thread(struct thread *td, void *dst, size_t *off)
> {
> +	struct arm64_addr_mask addr_mask;
> +	size_t len, mask_size;
> +
> +	len = 0;
> +	if (dst != NULL) {
> +		mask_size = sizeof(addr_mask);
> +		get_arm64_addr_mask(&regset_arm64_addr_mask, td, &addr_mask,
> +		    &mask_size);
> +
> +		len += elf64_populate_note(NT_ARM_ADDR_MASK, &addr_mask, dst,
> +		    sizeof(addr_mask), NULL);
> +	} else {
> +		len += elf64_populate_note(NT_ARM_ADDR_MASK, NULL, NULL,
> +		    sizeof(addr_mask), NULL);
> +	}
> 
> +	*off += len;
> }
> 
> bool
> diff --git a/sys/arm64/arm64/ptrauth.c b/sys/arm64/arm64/ptrauth.c
> index aa0591e351bb..5c129820cd37 100644
> --- a/sys/arm64/arm64/ptrauth.c
> +++ b/sys/arm64/arm64/ptrauth.c
> @@ -46,6 +46,8 @@ __FBSDID("$FreeBSD$");
> 
> #include <machine/armreg.h>
> #include <machine/cpu.h>
> +#include <machine/reg.h>
> +#include <machine/vmparam.h>
> 
> #define	SCTLR_PTRAUTH	(SCTLR_EnIA | SCTLR_EnIB | SCTLR_EnDA | SCTLR_EnDB)
> 
> @@ -82,8 +84,12 @@ ptrauth_init(void)
> 	 * it will also be available on any non-boot CPUs. If this is ever
> 	 * not the case we will have to add a quirk.
> 	 */
> -	if (ID_AA64ISAR1_APA_VAL(isar1) > 0 || ID_AA64ISAR1_API_VAL(isar1) > 0)
> +	if (ID_AA64ISAR1_APA_VAL(isar1) > 0 ||
> +	    ID_AA64ISAR1_API_VAL(isar1) > 0) {
> 		enable_ptrauth = true;
> +		elf64_addr_mask.code |= PAC_ADDR_MASK;
> +		elf64_addr_mask.data |= PAC_ADDR_MASK;
> +	}
> }
> 
> /* Copy the keys when forking a new process */
> diff --git a/sys/arm64/include/cpu.h b/sys/arm64/include/cpu.h
> index a6a606b58912..d3b13470a9be 100644
> --- a/sys/arm64/include/cpu.h
> +++ b/sys/arm64/include/cpu.h
> @@ -164,6 +164,9 @@ extern char etext[];
> 
> extern uint64_t __cpu_affinity[];
> 
> +struct arm64_addr_mask;
> +extern struct arm64_addr_mask elf64_addr_mask;
> +
> void	cpu_halt(void) __dead2;
> void	cpu_reset(void) __dead2;
> void	fork_trampoline(void);
> diff --git a/sys/arm64/include/reg.h b/sys/arm64/include/reg.h
> index bb151af55ba6..44b2e2b21b72 100644
> --- a/sys/arm64/include/reg.h
> +++ b/sys/arm64/include/reg.h
> @@ -83,6 +83,11 @@ struct dbreg32 {
> 	int dummy;
> };
> 
> +struct arm64_addr_mask {
> +	__uint64_t	code;
> +	__uint64_t	data;
> +};
> +
> #define	__HAVE_REG32
> 
> #endif /* !_MACHINE_REG_H_ */
> diff --git a/sys/arm64/include/vmparam.h b/sys/arm64/include/vmparam.h
> index 3e08b0a513b9..483c6d1f91a2 100644
> --- a/sys/arm64/include/vmparam.h
> +++ b/sys/arm64/include/vmparam.h
> @@ -156,6 +156,9 @@
> #define	VM_MIN_KERNEL_ADDRESS	(0xffff000000000000UL)
> #define	VM_MAX_KERNEL_ADDRESS	(0xffff008000000000UL)
> 
> +/* The address bits that hold a pointer authentication code */
> +#define	PAC_ADDR_MASK		(0xff7f000000000000UL)
> +
> /* If true addr is in the kernel address space */
> #define	ADDR_IS_KERNEL(addr)	(((addr) & (1ul << 55)) == (1ul << 55))
> /* If true addr is in its canonical form (i.e. no TBI, PAC, etc.) */
> diff --git a/sys/sys/elf_common.h b/sys/sys/elf_common.h
> index b8629fec8cbc..0bc93659adc5 100644
> --- a/sys/sys/elf_common.h
> +++ b/sys/sys/elf_common.h
> @@ -825,6 +825,7 @@ typedef struct {
> #define	NT_PPC_VSX	0x102	/* PowerPC VSX registers */
> #define	NT_X86_XSTATE	0x202	/* x86 XSAVE extended state. */
> #define	NT_ARM_VFP	0x400	/* ARM VFP registers */
> +#define	NT_ARM_ADDR_MASK	0x406	/* arm64 address mask (e.g. for TBI) */
> 
> /* GNU note types. */
> #define	NT_GNU_ABI_TAG		1
> diff --git a/tests/sys/kern/ptrace_test.c b/tests/sys/kern/ptrace_test.c
> index 3e3a445c6152..c62db66fd6f6 100644
> --- a/tests/sys/kern/ptrace_test.c
> +++ b/tests/sys/kern/ptrace_test.c
> @@ -3204,6 +3204,9 @@ ATF_TC_BODY(ptrace__PT_CONTINUE_with_signal_thread_sigmask, tc)
> ATF_TC_WITHOUT_HEAD(ptrace__PT_REGSET);
> ATF_TC_BODY(ptrace__PT_REGSET, tc)
> {
> +#if defined(__aarch64__)
> +	struct arm64_addr_mask addr_mask;
> +#endif
> 	struct prstatus prstatus;
> 	struct iovec vec;
> 	pid_t child, wpid;
> @@ -3242,6 +3245,16 @@ ATF_TC_BODY(ptrace__PT_REGSET, tc)
> 	ATF_REQUIRE(ptrace(PT_SETREGSET, wpid, (caddr_t)&vec, NT_PRSTATUS) !=
> 	    -1);
> 
> +#if defined(__aarch64__)
> +	vec.iov_base = &addr_mask;
> +	vec.iov_len = sizeof(addr_mask);
> +	ATF_REQUIRE(ptrace(PT_GETREGSET, wpid, (caddr_t)&vec,
> +	    NT_ARM_ADDR_MASK) != -1);
> +	REQUIRE_EQ(addr_mask.code, addr_mask.data);
> +	ATF_REQUIRE(addr_mask.code == 0 ||
> +	    addr_mask.code == 0xff7f000000000000UL);
> +#endif
> +
> 	REQUIRE_EQ(ptrace(PT_CONTINUE, child, (caddr_t)1, 0), 0);
> 
> 	/* The second wait() should report the exit status. */