1. Patchew
  2. Xen
  3. View series

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

Andrew Cooper posted 1 patch 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20250211125445.451805-1-andrew.cooper3@citrix.com
xen/arch/arm/arm32/traps.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
[PATCH for-4.20 v2] ARM32/traps: Fix do_trap_undefined_instruction()'s detection of kernel text
Posted by Andrew Cooper 1 month ago 
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);

failed 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
livepatches too.

Use is_active_kernel_text() which is the correct test for this purpose, and is
aware of init and livepatch regions as well as their lifetimes.

Fixes: 3e802c6ca1fb ("xen/arm: Correctly support WARN_ON")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
CC: Bertrand Marquis <bertrand.marquis@arm.com>
CC: Michal Orzel <michal.orzel@amd.com>
CC: Oleksii Kurochko <oleksii.kurochko@gmail.com>

v2:
 * Split out change to dump_execution_state()

Sample run going wrong:
  https://gitlab.com/xen-project/people/andyhhp/xen/-/jobs/9078570105

Sample run with dump_execution_state() working:
  https://gitlab.com/xen-project/people/andyhhp/xen/-/jobs/9079185111
---
 xen/arch/arm/arm32/traps.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/xen/arch/arm/arm32/traps.c b/xen/arch/arm/arm32/traps.c
index a2fc1c22cbc9..b88d41811b49 100644
--- a/xen/arch/arm/arm32/traps.c
+++ b/xen/arch/arm/arm32/traps.c
@@ -36,8 +36,7 @@ void do_trap_undefined_instruction(struct cpu_user_regs *regs)
     uint32_t pc = regs->pc;
     uint32_t instr;
 
-    if ( !is_kernel_text(pc) &&
-         (system_state >= SYS_STATE_active || !is_kernel_inittext(pc)) )
+    if ( !is_active_kernel_text(pc) )
         goto die;
 
     /* PC should be always a multiple of 4, as Xen is using ARM instruction set */
-- 
2.39.5
Re: [PATCH for-4.20 v2] ARM32/traps: Fix do_trap_undefined_instruction()'s detection of kernel text
Posted by Oleksii Kurochko 4 weeks ago 
On 2/11/25 1:54 PM, 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);
>
> failed 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
> livepatches too.
>
> Use is_active_kernel_text() which is the correct test for this purpose, and is
> aware of init and livepatch regions as well as their lifetimes.
>
> Fixes: 3e802c6ca1fb ("xen/arm: Correctly support WARN_ON")
> Signed-off-by: Andrew Cooper<andrew.cooper3@citrix.com>

Release-Acked-by:Oleksii Kurochko <oleksii.kurochko@gmail.com> ~ Oleksii

> ---
> CC: Stefano Stabellini<sstabellini@kernel.org>
> CC: Julien Grall<julien@xen.org>
> CC: Volodymyr Babchuk<Volodymyr_Babchuk@epam.com>
> CC: Bertrand Marquis<bertrand.marquis@arm.com>
> CC: Michal Orzel<michal.orzel@amd.com>
> CC: Oleksii Kurochko<oleksii.kurochko@gmail.com>
>
> v2:
>   * Split out change to dump_execution_state()
>
> Sample run going wrong:
>    https://gitlab.com/xen-project/people/andyhhp/xen/-/jobs/9078570105
>
> Sample run with dump_execution_state() working:
>    https://gitlab.com/xen-project/people/andyhhp/xen/-/jobs/9079185111
> ---
>   xen/arch/arm/arm32/traps.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/arm32/traps.c b/xen/arch/arm/arm32/traps.c
> index a2fc1c22cbc9..b88d41811b49 100644
> --- a/xen/arch/arm/arm32/traps.c
> +++ b/xen/arch/arm/arm32/traps.c
> @@ -36,8 +36,7 @@ void do_trap_undefined_instruction(struct cpu_user_regs *regs)
>       uint32_t pc = regs->pc;
>       uint32_t instr;
>   
> -    if ( !is_kernel_text(pc) &&
> -         (system_state >= SYS_STATE_active || !is_kernel_inittext(pc)) )
> +    if ( !is_active_kernel_text(pc) )
>           goto die;
>   
>       /* PC should be always a multiple of 4, as Xen is using ARM instruction set */
Re: [PATCH for-4.20 v2] ARM32/traps: Fix do_trap_undefined_instruction()'s detection of kernel text
Posted by Julien Grall 4 weeks ago 
Hi Andrew,

On 11/02/2025 12:54, 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);
> 
> failed 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
> livepatches too.
> 
> Use is_active_kernel_text() which is the correct test for this purpose, and is
> aware of init and livepatch regions as well as their lifetimes.
> 
> Fixes: 3e802c6ca1fb ("xen/arm: Correctly support WARN_ON")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Julien Grall <jgrall@amazon.com>

Cheers,

-- 
Julien Grall

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.