Re: [PATCH v2 07/10] libfdt: Handle unknown tags in fdt_next_tag()

On Thu, Apr 09, 2026 at 01:54:23PM +0200, Herve Codina wrote:
> The structured tag value definition introduced recently gives the
> ability to ignore unknown tags without any error when they are read.
>
> libfdt uses fdt_next_tag() to get a tag.
>
> Filtering out tags that should be ignored in fdt_next_tag() allows to
> have the filtering done globally and allows, in future releases, to have
> a central place to add new known tags that should not be filtered out.
>
> An already known tag exists with the meaning of "just ignore". This tag
> is FDT_NOP. fdt_next_tag() callers already handle the FDT_NOP tag.
>
> Avoid unneeded modification at callers side and use a fake FDT_NOP tag
> when an unknown tag that should be ignored is encountered.
>
> Add also fdt_next_tag_() internal function for callers who need to know
> if the FDT_NOP tag returned is a real FDT_NOP or a fake FDT_NOP due to
> an unknown tag.
>
> Signed-off-by: Herve Codina <[email protected]>
> Reviewed-by: Luca Ceresoli <[email protected]>
> ---

Reviewed-by: Frank Li <[email protected]>

>  libfdt/fdt.c             | 75 ++++++++++++++++++++++++++++++++++++++--
>  libfdt/libfdt_internal.h |  3 ++
>  tests/run_tests.sh       |  9 +++--
>  3 files changed, 83 insertions(+), 4 deletions(-)
>
> diff --git a/libfdt/fdt.c b/libfdt/fdt.c
> index fb4faba..cce1373 100644
> --- a/libfdt/fdt.c
> +++ b/libfdt/fdt.c
> @@ -167,7 +167,7 @@ const void *fdt_offset_ptr(const void *fdt, int offset, unsigned int len)
>  	return fdt_offset_ptr_(fdt, offset);
>  }
>
> -uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset)
> +static uint32_t fdt_next_tag_all(const void *fdt, int startoffset, int *nextoffset)
>  {
>  	const fdt32_t *tagp, *lenp;
>  	uint32_t tag, len, sum;
> @@ -218,7 +218,37 @@ uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset)
>  		break;
>
>  	default:
> -		return FDT_END;
> +		if (!(tag & FDT_TAG_STRUCTURED) || !(tag & FDT_TAG_SKIP_SAFE))
> +			return FDT_END;
> +
> +		switch (tag & FDT_TAG_DATA_MASK) {
> +		case FDT_TAG_DATA_NONE:
> +			break;
> +		case FDT_TAG_DATA_1CELL:
> +			offset += FDT_CELLSIZE;
> +			break;
> +		case FDT_TAG_DATA_2CELLS:
> +			offset += 2 * FDT_CELLSIZE;
> +			break;
> +		case FDT_TAG_DATA_VARLEN:
> +			/* Get the length */
> +			lenp = fdt_offset_ptr(fdt, offset, sizeof(*lenp));
> +			if (!can_assume(VALID_DTB) && !lenp)
> +				return FDT_END; /* premature end */
> +			len = fdt32_to_cpu(*lenp);
> +			/*
> +			 * Skip the cell encoding the length and the
> +			 * following length bytes
> +			 */
> +			len += sizeof(*lenp);
> +			sum = len + offset;
> +			if (!can_assume(VALID_DTB) &&
> +			    (sum >= INT_MAX || sum < (uint32_t) offset))
> +				return FDT_END; /* premature end */
> +
> +			offset += len;
> +			break;
> +		}
>  	}
>
>  	if (!fdt_offset_ptr(fdt, startoffset, offset - startoffset))
> @@ -228,6 +258,47 @@ uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset)
>  	return tag;
>  }
>
> +static bool fdt_is_unknown_tag(uint32_t tag)
> +{
> +	switch (tag) {
> +	case FDT_BEGIN_NODE:
> +	case FDT_END_NODE:
> +	case FDT_PROP:
> +	case FDT_NOP:
> +	case FDT_END:
> +		return false;
> +	default:
> +		break;
> +	}
> +	return true;
> +}
> +
> +uint32_t fdt_next_tag_(const void *fdt, int startoffset, int *nextoffset, bool *is_unknown)
> +{
> +	uint32_t tag;
> +	bool unknown = false;
> +
> +	/* Retrieve next tag */
> +	tag = fdt_next_tag_all(fdt, startoffset, nextoffset);
> +	if (tag == FDT_END)
> +		goto end;
> +
> +	if (fdt_is_unknown_tag(tag)) {
> +		unknown = true;
> +		/* Use a known tag that should be skipped by the caller */
> +		tag = FDT_NOP;
> +	}
> +end:
> +	if (is_unknown)
> +		*is_unknown = unknown;
> +	return tag;
> +}
> +
> +uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset)
> +{
> +	return fdt_next_tag_(fdt, startoffset, nextoffset, NULL);
> +}
> +
>  int fdt_check_node_offset_(const void *fdt, int offset)
>  {
>  	if (!can_assume(VALID_INPUT)
> diff --git a/libfdt/libfdt_internal.h b/libfdt/libfdt_internal.h
> index 4c15264..c1ae306 100644
> --- a/libfdt/libfdt_internal.h
> +++ b/libfdt/libfdt_internal.h
> @@ -20,6 +20,9 @@ int32_t fdt_ro_probe_(const void *fdt);
>  		}							\
>  	}
>
> +uint32_t fdt_next_tag_(const void *fdt, int startoffset, int *nextoffset,
> +		       bool *is_unknown);
> +
>  int fdt_check_node_offset_(const void *fdt, int offset);
>  int fdt_check_prop_offset_(const void *fdt, int offset);
>
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index d147011..48ac6fa 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -576,11 +576,12 @@ libfdt_tests () {
>      run_test dtbs_equal_ordered cell-overflow.test.dtb cell-overflow-results.test.dtb
>
>      # check full tests
> -    for good in test_tree1.dtb; do
> +    for good in test_tree1.dtb unknown_tags_can_skip.dtb; do
>  	run_test check_full $good
>      done
>      for bad in truncated_property.dtb truncated_string.dtb \
> -		truncated_memrsv.dtb two_roots.dtb named_root.dtb; do
> +		truncated_memrsv.dtb two_roots.dtb named_root.dtb \
> +		unknown_tags_no_skip.dtb; do
>  	run_test check_full -n $bad
>      done
>  }
> @@ -961,6 +962,10 @@ fdtget_tests () {
>      run_fdtget_test "<the dead silence>" -tx \
>  	-d "<the dead silence>" $dtb /randomnode doctor-who
>      run_fdtget_test "<blink>" -tx -d "<blink>" $dtb /memory doctor-who
> +
> +    # test with unknown tags involved
> +    run_fdtget_test "25601 25602" unknown_tags_can_skip.dtb /subnode1 prop-int
> +    run_wrap_error_test $DTGET unknown_tags_no_skip.dtb /subnode1 prop-int
>  }
>
>  fdtput_tests () {
> --
> 2.53.0
>