[PATCH v20] arm64: expose FAR_EL1 tag bits in siginfo
Eric W. Biederman
ebiederm at xmission.com
Fri Nov 20 12:43:35 EST 2020
Peter Collingbourne <pcc at google.com> writes:
> The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
> address exposed via siginfo.si_addr and sigcontext.fault_address. However,
> the tag bits may be needed by tools in order to accurately diagnose
> memory errors, such as HWASan [1] or future tools based on the Memory
> Tagging Extension (MTE).
>
> We should not stop clearing these bits in the existing fault address
> fields, because there may be existing userspace applications that are
> expecting the tag bits to be cleared. Instead, introduce a flag in
> sigaction.sa_flags, SA_EXPOSE_TAGBITS, and only expose the tag bits
> there if the signal handler has this flag set.
>
> [1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html
For the generic bits:
Acked-by: "Eric W. Biederman" <ebiederm at xmission.com>
Some of the arm bits look wrong. There are a couple of cases where
it looks like you are deliberately passing an untagged address into
functions that normally take tagged addresses.
It might be a good idea to have a type distinction between the two.
Perhaps "(void __user *)" vs "(unsigned long)" so that accidentally
using the wrong one generates a type error.
I don't think I am really qualified to review all of the arm details,
and I certainly don't want to be in the middle of any arm bugs this
code might introduce. If you will split out the generic bits of this
patch I will take it. The this whole thing can be merged into the arm
tree and you can ensure the arm bits are correct.
Eric
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index 1ee94002801f..c5375cb7763d 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -596,33 +596,35 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
> return 0;
> }
>
> -static int __kprobes do_translation_fault(unsigned long addr,
> +static int __kprobes do_translation_fault(unsigned long far,
> unsigned int esr,
> struct pt_regs *regs)
> {
> + unsigned long addr = untagged_addr(far);
> +
> if (is_ttbr0_addr(addr))
> - return do_page_fault(addr, esr, regs);
> + return do_page_fault(far, esr, regs);
>
> - do_bad_area(addr, esr, regs);
> + do_bad_area(far, esr, regs);
> return 0;
> }
>
> -static int do_alignment_fault(unsigned long addr, unsigned int esr,
> +static int do_alignment_fault(unsigned long far, unsigned int esr,
> struct pt_regs *regs)
> {
> - do_bad_area(addr, esr, regs);
> + do_bad_area(far, esr, regs);
> return 0;
> }
>
> -static int do_bad(unsigned long addr, unsigned int esr, struct pt_regs *regs)
> +static int do_bad(unsigned long far, unsigned int esr, struct pt_regs *regs)
> {
> return 1; /* "fault" */
> }
>
> -static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
> +static int do_sea(unsigned long far, unsigned int esr, struct pt_regs *regs)
> {
> const struct fault_info *inf;
> - void __user *siaddr;
> + unsigned long siaddr;
>
> inf = esr_to_fault_info(esr);
>
> @@ -635,18 +637,23 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
> }
>
> if (esr & ESR_ELx_FnV)
> - siaddr = NULL;
> + siaddr = 0;
> else
> - siaddr = (void __user *)addr;
> + siaddr = untagged_addr(far);
> arm64_notify_die(inf->name, regs, inf->sig, inf->code, siaddr, esr);
>
What is going on in this function?
Are you deliberately removing the tag bits?
> return 0;
> }
>
> -static int do_tag_check_fault(unsigned long addr, unsigned int esr,
> +static int do_tag_check_fault(unsigned long far, unsigned int esr,
> struct pt_regs *regs)
> {
> - do_bad_area(addr, esr, regs);
> + /*
> + * The architecture specifies that bits 63:60 of FAR_EL1 are UNKNOWN for tag
> + * check faults. Mask them out now so that userspace doesn't see them.
> + */
> + far &= (1UL << 60) - 1;
> + do_bad_area(far, esr, regs);
> return 0;
> }
>
> @@ -717,11 +724,12 @@ static const struct fault_info fault_info[] = {
> { do_bad, SIGKILL, SI_KERNEL, "unknown 63" },
> };
>
> -void do_mem_abort(unsigned long addr, unsigned int esr, struct pt_regs *regs)
> +void do_mem_abort(unsigned long far, unsigned int esr, struct pt_regs *regs)
> {
> const struct fault_info *inf = esr_to_fault_info(esr);
> + unsigned long addr = untagged_addr(far);
>
> - if (!inf->fn(addr, esr, regs))
> + if (!inf->fn(far, esr, regs))
> return;
>
> if (!user_mode(regs)) {
> @@ -730,8 +738,7 @@ void do_mem_abort(unsigned long addr, unsigned int esr, struct pt_regs *regs)
> show_pte(addr);
> }
>
> - arm64_notify_die(inf->name, regs,
> - inf->sig, inf->code, (void __user *)addr, esr);
> + arm64_notify_die(inf->name, regs, inf->sig, inf->code, addr, esr);
What is going on in this function?
Are you deliberately removing the tag bits?
> }
> NOKPROBE_SYMBOL(do_mem_abort);
>
More information about the linux-arm-kernel
mailing list