[PATCH] libfdt: Validate alias property value is a valid string.

[PATCH] libfdt: Validate alias property value is a valid string.

[Mike McTernan]

Prevent circular alias resolution causing infinite recursion.

Signed-off-by: Mike McTernan <mikemcternan-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
 libfdt/fdt_ro.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
index 9f6c551..870c4a5 100644
--- a/libfdt/fdt_ro.c
+++ b/libfdt/fdt_ro.c
@@ -525,13 +525,27 @@ uint32_t fdt_get_phandle(const void *fdt, int nodeoffset)
 const char *fdt_get_alias_namelen(const void *fdt,
 				  const char *name, int namelen)
 {
+	const char *prop;
 	int aliasoffset;
+	int prop_len;
 
 	aliasoffset = fdt_path_offset(fdt, "/aliases");
 	if (aliasoffset < 0)
 		return NULL;
 
-	return fdt_getprop_namelen(fdt, aliasoffset, name, namelen, NULL);
+	prop = fdt_getprop_namelen(fdt, aliasoffset, name, namelen, &prop_len);
+	if (prop && !can_assume(VALID_INPUT)) {
+		/* Validate the alias value.  From the devicetree spec v0.3:
+		 * "An alias value is a device path and is encoded as a string.
+		 *  The value representes the full path to a node, ..."
+		 * A full path must start at the root to prevent recursion.
+		 */
+		if (prop_len < 2 || *prop != '/' || strnlen(prop, prop_len) != prop_len - 1) {
+			prop = NULL;
+		}
+	}
+
+	return prop;
 }
 
 const char *fdt_get_alias(const void *fdt, const char *name)
-- 
2.38.0.rc2.412.g84df46c1b4-goog

Re: [PATCH] libfdt: Validate alias property value is a valid string.

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2296 bytes --]

On Mon, Oct 10, 2022 at 11:03:28AM +0100, Mike McTernan wrote:
> Prevent circular alias resolution causing infinite recursion.

Sorry, I've neglected this forever.  Unfortunately, I'm not really
sure what to do with it.  On the one hand, this does prevent infinite
recursion which supports the libfdt design goal of being robust
against being given bad trees.

However, although the modern device spec does say "full path"
(although I'm not sure it spells out exactly what that means), I have
a fairly strong memory that IEEE 1275 did allow aliases to reference
one another.  Ideally I'd like libfdt to be able to handle trees which
are transcribed from Open Firmware as well as more modern dtbs created
with dtc.

> Signed-off-by: Mike McTernan <mikemcternan-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> ---
>  libfdt/fdt_ro.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
> index 9f6c551..870c4a5 100644
> --- a/libfdt/fdt_ro.c
> +++ b/libfdt/fdt_ro.c
> @@ -525,13 +525,27 @@ uint32_t fdt_get_phandle(const void *fdt, int nodeoffset)
>  const char *fdt_get_alias_namelen(const void *fdt,
>  				  const char *name, int namelen)
>  {
> +	const char *prop;
>  	int aliasoffset;
> +	int prop_len;
>  
>  	aliasoffset = fdt_path_offset(fdt, "/aliases");
>  	if (aliasoffset < 0)
>  		return NULL;
>  
> -	return fdt_getprop_namelen(fdt, aliasoffset, name, namelen, NULL);
> +	prop = fdt_getprop_namelen(fdt, aliasoffset, name, namelen, &prop_len);
> +	if (prop && !can_assume(VALID_INPUT)) {
> +		/* Validate the alias value.  From the devicetree spec v0.3:
> +		 * "An alias value is a device path and is encoded as a string.
> +		 *  The value representes the full path to a node, ..."
> +		 * A full path must start at the root to prevent recursion.
> +		 */
> +		if (prop_len < 2 || *prop != '/' || strnlen(prop, prop_len) != prop_len - 1) {
> +			prop = NULL;
> +		}
> +	}
> +
> +	return prop;
>  }
>  
>  const char *fdt_get_alias(const void *fdt, const char *name)

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]