Re: [PATCH v2 02/10] libfdt: Don't assume that a FDT_BEGIN_NODE tag is available at offset 0

On Thu, Apr 09, 2026 at 01:54:18PM +0200, Herve Codina wrote:
> In several places, libfdt assumes that a FDT_BEGIN_NODE tag is present
> at the offset 0 of the structure block.
>
> This assumption is not correct. Indeed, a FDT_NOP can be present at the
> offset 0 and this is a legit case.
>
> fdt_first_node() has been introduced recently to get the offset of the
> first node (first FDT_BEGIN_NODE) in a fdt blob.
>
> Use this function to get the first node offset instead of looking for
> this node at offset 0.
>
> Signed-off-by: Herve Codina <[email protected]>
> ---
>  libfdt/fdt.c    | 14 ++++++++++++--
>  libfdt/fdt_ro.c | 16 +++++++++++++---
>  libfdt/fdt_rw.c |  6 ++++++
>  3 files changed, 31 insertions(+), 5 deletions(-)
>
> diff --git a/libfdt/fdt.c b/libfdt/fdt.c
> index 676c7d7..fb4faba 100644
> --- a/libfdt/fdt.c
> +++ b/libfdt/fdt.c
> @@ -279,11 +279,21 @@ int fdt_first_node(const void *fdt)
>
>  int fdt_next_node(const void *fdt, int offset, int *depth)
>  {
> -	int nextoffset = 0;
> +	int nextoffset = offset;
>  	uint32_t tag;
>
> +	/*
> +	 * Get the first node if asked for next node from the first node
> +	 * (offset == 0) or if the given offset is not valid (negative).
> +	 */
> +	if (offset <= 0) {
> +		nextoffset = fdt_first_node(fdt);
> +		if (nextoffset < 0)
> +			return nextoffset;
> +	}
> +
>  	if (offset >= 0)
> -		if ((nextoffset = fdt_check_node_offset_(fdt, offset)) < 0)
> +		if ((nextoffset = fdt_check_node_offset_(fdt, nextoffset)) < 0)
>  			return nextoffset;
>
>  	do {
> diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
> index 63494fb..8e1db7d 100644
> --- a/libfdt/fdt_ro.c
> +++ b/libfdt/fdt_ro.c
> @@ -229,6 +229,12 @@ int fdt_subnode_offset_namelen(const void *fdt, int offset,
>
>  	FDT_RO_PROBE(fdt);
>
> +	if (!offset) {

I am no sure if corrrect, but previous block, you use if (offset <= 0),
same below fdt_add_subnode_namelen()

Frank

> +		offset = fdt_first_node(fdt);
> +		if (offset < 0)
> +			return offset;
> +	}
> +
>  	for (depth = 0;
>  	     (offset >= 0) && (depth >= 0);
>  	     offset = fdt_next_node(fdt, offset, &depth))
> @@ -251,13 +257,17 @@ int fdt_path_offset_namelen(const void *fdt, const char *path, int namelen)
>  {
>  	const char *end = path + namelen;
>  	const char *p = path;
> -	int offset = 0;
> +	int offset;
>
>  	FDT_RO_PROBE(fdt);
>
>  	if (!can_assume(VALID_INPUT) && namelen <= 0)
>  		return -FDT_ERR_BADPATH;
>
> +	offset = fdt_first_node(fdt);
> +	if (offset < 0)
> +		return offset;
> +
>  	/* see if we have an alias */
>  	if (*path != '/') {
>  		const char *q = memchr(path, '/', end - p);
> @@ -579,7 +589,7 @@ int fdt_get_path(const void *fdt, int nodeoffset, char *buf, int buflen)
>  	if (buflen < 2)
>  		return -FDT_ERR_NOSPACE;
>
> -	for (offset = 0, depth = 0;
> +	for (offset = fdt_first_node(fdt), depth = 0;
>  	     (offset >= 0) && (offset <= nodeoffset);
>  	     offset = fdt_next_node(fdt, offset, &depth)) {
>  		while (pdepth > depth) {
> @@ -617,7 +627,7 @@ int fdt_get_path(const void *fdt, int nodeoffset, char *buf, int buflen)
>  	else if (offset == -FDT_ERR_BADOFFSET)
>  		return -FDT_ERR_BADSTRUCTURE;
>
> -	return offset; /* error from fdt_next_node() */
> +	return offset; /* error from fdt_next_node() or fdt_first_node() */
>  }
>
>  int fdt_supernode_atdepth_offset(const void *fdt, int nodeoffset,
> diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c
> index 90ea14e..f5c28fc 100644
> --- a/libfdt/fdt_rw.c
> +++ b/libfdt/fdt_rw.c
> @@ -354,6 +354,12 @@ int fdt_add_subnode_namelen(void *fdt, int parentoffset,
>
>  	FDT_RW_PROBE(fdt);
>
> +	if (!parentoffset) {
> +		parentoffset = fdt_first_node(fdt);
> +		if (parentoffset < 0)
> +			return parentoffset;
> +	}
> +
>  	offset = fdt_subnode_offset_namelen(fdt, parentoffset, name, namelen);
>  	if (offset >= 0)
>  		return -FDT_ERR_EXISTS;
> --
> 2.53.0
>