Re: [PATCH] ARM: xen: validate hypervisor compatible before parsing its version

[Stefano Stabellini]

On Fri, 3 Apr 2026, Pengpeng Hou wrote:
> fdt_find_hyper_node() reads the raw compatible property and then
> derives hyper_node.version from a prefix match before later printing it
> with %s. Flat DT properties are external boot input, and this path does
> not prove that the compatible string is NUL-terminated within its
> declared bounds.
> 
> Fetch the first compatible entry with fdt_stringlist_get() so malformed
> unterminated properties are rejected before the version suffix is
> parsed.
> 
> Signed-off-by: Pengpeng Hou <pengpeng@xxxxxxxxxxx>
> ---
>  arch/arm/xen/enlighten.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index 4feed2c2498d..f69290a4c639 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -19,6 +19,7 @@
>  #include <asm/efi.h>
>  #include <linux/interrupt.h>
>  #include <linux/irqreturn.h>
> +#include <linux/libfdt.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_fdt.h>
> @@ -218,8 +219,9 @@ static __initdata struct {
>  static int __init fdt_find_hyper_node(unsigned long node, const char *uname,
>  				      int depth, void *data)
>  {
> -	const void *s = NULL;
> +	const char *s = NULL;
>  	int len;
> +	size_t prefix_len = strlen(hyper_node.prefix);
>  
>  	if (depth != 1 || strcmp(uname, "hypervisor") != 0)
>  		return 0;
> @@ -227,10 +229,10 @@ static int __init fdt_find_hyper_node(unsigned long node, const char *uname,
>  	if (of_flat_dt_is_compatible(node, hyper_node.compat))
>  		hyper_node.found = true;
>  
> -	s = of_get_flat_dt_prop(node, "compatible", &len);
> -	if (strlen(hyper_node.prefix) + 3  < len &&
> -	    !strncmp(hyper_node.prefix, s, strlen(hyper_node.prefix)))
> -		hyper_node.version = s + strlen(hyper_node.prefix);
> +	s = fdt_stringlist_get(initial_boot_params, node, "compatible", 0, &len);
> +	if (s && len > prefix_len + 2 &&
> +	    !strncmp(hyper_node.prefix, s, prefix_len))
> +		hyper_node.version = s + prefix_len;

I'd prefer to go with:

  s = of_get_flat_dt_prop(node, "compatible", &len);
  if (s && len > 0 && strnlen(s, len) < len &&
      len > prefix_len + 3 &&
      !strncmp(hyper_node.prefix, s, prefix_len))