Re: problem while handling exceptions in oob stage

[Jan Kiszka <jan.kiszka@siemens.com>]

On 23.02.23 14:22, Johannes Kirchmair wrote:
> Hello,
> 
> we encountered a problem, while handling invalid opcode exception in oob stage. 
> Our fixup code works fine if we execute ud2 instruction the first time.
> Upon second execution of ud2, we encounter the problem that our fixup code is not reached.
> 
> The problem is that the first time entering the kernel, mark_trap_entry_raw is called and in a subsequent call the _TLF_OOBTRAP flag is set.
> But because we do not switch to inband stage, we do not call mark_trap_exit_raw and the _TLF_OOBTRAP flag is not cleared.
> 
> The second time we enter the kernel on execution of ud2,  we encounter (again) the following code:
> 
> static __always_inline void oob_trap_notify(unsigned int exception,
>                                          struct pt_regs *regs)
> {
>          if (running_oob() && !test_thread_local_flags(_TLF_OOBTRAP))
>                  __oob_trap_notify(exception, regs);
> }
> 
> Because _TLF_OOBTRAP is set and because we are still oob, we just leave the kernel without calling __oob_trap_notify and subsequently our clean up code.
> This results in re-execution of the same ud2 again and again.
> 
> I fixed it for me in the following way:
> 
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -414,8 +414,9 @@ DEFINE_IDTENTRY_RAW(exc_invalid_op)
>                 handle_invalid_op(regs);
>                 instrumentation_end();
>                 irqentry_exit(regs, state);
> -               mark_trap_exit_raw(X86_TRAP_UD, regs);
>         }
> +
> +       mark_trap_exit_raw(X86_TRAP_UD, regs);
>  }
> 
> I am not sure it this is a valid way to fix this issue.
> If it is, I could work up a patch next week that addresses all the places where this problem could trigger.
> 
> What do you think?
> 

Good catch. Looking at my old x86 patch, I was calling mark_trap_exit in
all cases as well.

Still, we should check what happens here if it is semantically correct.
When I look at __oob_trap_unwind which is now being call for both exits,
it contains the surely desired clearing of _TLF_OOBTRAP but also the
invocation of the handle_oob_trap_exit. That is harmless for Xenomai as
we don't use it, but maybe it is not desired. Philippe needs to comment
on this.

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux