Re: [PATCH for-4.20? 1/4] ARM32/traps: Fix do_trap_undefined_instruction()'s detection of kernel text

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 10/02/2025 10:13 am, Orzel, Michal wrote:
>
> On 08/02/2025 01:02, Andrew Cooper wrote:
>>
>> While fixing some common/arch boundaries for UBSAN support on other
>> architectures, the following debugging patch:
>>
>>   diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>>   index c1f2d1b89d43..58d1d048d339 100644
>>   --- a/xen/arch/arm/setup.c
>>   +++ b/xen/arch/arm/setup.c
>>   @@ -504,6 +504,8 @@ void asmlinkage __init start_xen(unsigned long 
>> fdt_paddr)
>>
>>        system_state = SYS_STATE_active;
>>
>>   +    dump_execution_state();
>>   +
>>        for_each_domain( d )
>>            domain_unpause_by_systemcontroller(d);
>>
>> fails with:
>>
>>   (XEN) *** Serial input to DOM0 (type 'CTRL-a' three times to switch input)
>>   (XEN) CPU0: Unexpected Trap: Undefined Instruction
>>   (XEN) ----[ Xen-4.20-rc  arm32  debug=n  Not tainted ]----
>>   (XEN) CPU:    0
>>   <snip>
>>   (XEN)
>>   (XEN) ****************************************
>>   (XEN) Panic on CPU 0:
>>   (XEN) CPU0: Unexpected Trap: Undefined Instruction
>>   (XEN) ****************************************
>>
>> This is because the condition for init text is wrong.  While there's nothing
>> interesting from that point onwards in start_xen(), it's also wrong for any
>> livepatch which brings in an adjusted BUG_FRAME().
>>
>> Use is_active_kernel_text() which is the correct test for this purpose, and 
>> is
>> aware of init and livepatch regions too.
>>
>> Commit c8d4b6304a5e ("xen/arm: add support for run_in_exception_handler()"),
>> made run_in_exception_handler() work, but didn't complete the TODO left in
>> commit 3e802c6ca1fb ("xen/arm: Correctly support WARN_ON").  Do so, to make
>> ARM consistent with other architectures.
>>
>> Fixes: 3e802c6ca1fb ("xen/arm: Correctly support WARN_ON")
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> You should have mentioned that this patch requires [1] as a prerequisite.
> Otherwise this patch fails to build on both arm64 and arm32 with UBSAN 
> enabled.
>
> [1]
> https://lore.kernel.org/xen-devel/359347d3-9a5f-4672-98d6-4c497d960059@xxxxxxxxx/T/#mc75e1b1ff6ccf4b0c7e10f55eedb7cacffca1c3d

That is unintentional.

I'm going to split this patch in half, because it's clear that the
run_in_exception_handler() problems are more complicated than I expected.

The fix in do_trap_undefined_instruction() genuinely is entirely
independent of UBSAN.

~Andrew