Re: [dovetail][PATCH] arm64: dovetail: Notify the core about compat alignment fixups

[Philippe Gerum <rpm@xenomai.org>]

Jan Kiszka <jan.kiszka@siemens.com> writes:

> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> With CONFIG_COMPAT_ALIGNMENT_FIXUPS, misaligned multi-word loads and
> stores of 32-bit tasks are emulated by do_compat_alignment_fixup(),
> called from do_alignment_fault() via do_mem_abort(). The companion
> core is only told about the trap when the fixup fails and
> do_mem_abort() falls back to signal injection. If the fixup succeeds,
> an oob thread runs the whole emulation from the oob stage: it fetches
> the faulting instruction, emulates it by accessing user memory, which
> may fault in pages, and accounts a perf event, none of which is
> oob-safe. On arm, the equivalent path in do_alignment() is annotated.
>
> Notify the core before attempting the fixup, reporting the trap as
> ARM64_TRAP_ACCESS like do_bad_area() does for the same fault when the
> fixups are disabled. If the fixup fails, the notification is unwound
> before do_mem_abort() goes on, so its own annotation pairs as usual.
>
> Assisted-by: opencode:Opus-5.5
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>
> This was a found by pure code analysis as most setups have
> CONFIG_COMPAT_ALIGNMENT_FIXUPS off. It was not tested yet, working on a 
> setup.
>
>  arch/arm64/mm/fault.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index 2726eb3fa2d46..c3532ff0440ad 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -867,9 +867,15 @@ static int __kprobes do_translation_fault(unsigned long far,
>  static int do_alignment_fault(unsigned long far, unsigned long esr,
>  			      struct pt_regs *regs)
>  {
> +	int ret;
> +
>  	if (IS_ENABLED(CONFIG_COMPAT_ALIGNMENT_FIXUPS) &&
> -	    compat_user_mode(regs))
> -		return do_compat_alignment_fixup(far, regs);
> +	    compat_user_mode(regs)) {
> +		mark_trap_entry(ARM64_TRAP_ACCESS, regs);
> +		ret = do_compat_alignment_fixup(far, regs);
> +		mark_trap_exit(ARM64_TRAP_ACCESS, regs);
> +		return ret;
> +	}
>  	do_bad_area(far, esr, regs);
>  	return 0;
>  }

If oob, we can expect MMF_DOVETAIL to be set for the current address
space, so .text pages of the caller should have been committed. In the
(unexpected) event some are not, get_user() and friends would still
recursively bump us to do_page_fault()/do_translation_fault() on a
subsequent synchronous exception path, which would mark the trap entry
eventually, causing the core to downgrade the task before damage is
done. This should work now that pipelined faults can pile up
safely. Anyway, I'll wait for some testing, things might be more
complicated.

-- 
Philippe.