Re: [PATCH] ubsan: Introduce CONFIG_UBSAN_FATAL to panic on UBSAN failure

[Julien Grall <julien@xxxxxxx>]

Hi Michal,

On 27/11/2023 15:41, Michal Orzel wrote:
> Introduce the CONFIG_UBSAN_FATAL option to cater to scenarios where prompt
> attention to undefined behavior issues, notably during CI test runs, is
> essential. When enabled, this option causes Xen to panic upon detecting
> UBSAN failure (as the last step in ubsan_epilogue()).
I have mixed opinions on this patch. This would be a good one if we had 
a Xen mostly free from UBSAN behavior. But this is not the case at least 
on arm32. So we would end up to stop at the first error. IOW, we would 
need to fix the first error before we can see the next one.
So I feel it would be best if the gitlab CI jobs actually check for 
USBAN in the logs and fail if there are any. With that, we can get the 
full list of UBSAN issues on each job.
Cheers,

> Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
> ---
>   xen/Kconfig.debug        | 7 +++++++
>   xen/common/ubsan/ubsan.c | 4 ++++
>   2 files changed, 11 insertions(+)
>
> diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
> index 94e818ee09b5..e19e9d48817c 100644
> --- a/xen/Kconfig.debug
> +++ b/xen/Kconfig.debug
> @@ -107,6 +107,13 @@ config UBSAN
>   
>   	  If unsure, say N here.
>   
> +config UBSAN_FATAL
> +       bool "Panic on UBSAN failure"
> +       depends on UBSAN
> +       help
> +         Enabling this option will cause Xen to panic when an undefined 
> behavior
> +         is detected by UBSAN. If unsure, say N here.
> +
>   config DEBUG_TRACE
>         bool "Debug trace support"
>         ---help---
> diff --git a/xen/common/ubsan/ubsan.c b/xen/common/ubsan/ubsan.c
> index a3a80fa99eec..dd5ee0013648 100644
> --- a/xen/common/ubsan/ubsan.c
> +++ b/xen/common/ubsan/ubsan.c
> @@ -174,6 +174,10 @@ static void ubsan_epilogue(unsigned long *flags)
>                 "========================================\n");
>         spin_unlock_irqrestore(&report_lock, *flags);
>         current->in_ubsan--;
> +
> +#ifdef CONFIG_UBSAN_FATAL
> +       panic("UBSAN failure detected\n");
> +#endif
>   }
>   
>   static void handle_overflow(struct overflow_data *data, unsigned long lhs,
--
Julien Grall