[PATCH v2 05/10] libfdt: overlay: Prepare fixup and fragment path helpers for reuse

[Douglas Anderson <dianders@chromium.org>]

Clean up `overlay_parse_fixup_entry()` and generalize
`overlay_split_fragment_path()` in `libfdt/fdt_overlay.c` so both can be
reused when updating `/__local_fixups__`:

- Change `poffset` from `int` to `uint32_t` in
  `overlay_fixup_one_phandle()`, `overlay_parse_fixup_entry()`, and
  `overlay_fixup_phandle()`, and parse into `unsigned long` before
  validating the range.
- Require `*(sep + 1)` to be an ASCII digit (`'0'..'9'`) and bound
  `poffset` to `INT32_MAX - sizeof(uint32_t)` so negative strings (such
  as `"-4"`), leading whitespace, or large values cannot silently
  truncate or wrap around in `fdt_setprop_inplace_namelen_partial()`.
- Drop the redundant `*sep != ':'` checks after `memchr(..., ':', ...)`.
- Generalize `overlay_split_fragment_path()` to accept a length-bounded
  `path` slice that is not necessarily NUL-terminated (using `memchr()`
  instead of `strchr()`, and keeping the NUL-terminated property check
  in `overlay_symbol_update()`) so it can also split `:`-delimited paths
  from `/__fixups__`.

Assisted-by: Gemini:gemini-pro-prerelease
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

Changes in v2:
- New patch tightening fixup offset parsing and generalizing
  overlay_split_fragment_path() for non-NUL-terminated paths in
  libfdt/fdt_overlay.c

 libfdt/fdt_overlay.c | 46 +++++++++++++++++++++++---------------------
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/libfdt/fdt_overlay.c b/libfdt/fdt_overlay.c
index f9d1a555ed5d..eb87b356145f 100644
--- a/libfdt/fdt_overlay.c
+++ b/libfdt/fdt_overlay.c
@@ -330,7 +330,7 @@ static int overlay_update_local_references(void *fdto, uint32_t delta)
 static int overlay_fixup_one_phandle(void *fdto, int symbols_off,
 				     const char *path, uint32_t path_len,
 				     const char *name, uint32_t name_len,
-				     int poffset, uint32_t phandle)
+				     uint32_t poffset, uint32_t phandle)
 {
 	fdt32_t phandle_prop;
 	int fixup_off;
@@ -368,7 +368,7 @@ static int overlay_fixup_one_phandle(void *fdto, int symbols_off,
 static int overlay_parse_fixup_entry(const char **valuep, int *lenp,
 				     const char **pathp, uint32_t *path_lenp,
 				     const char **namep, uint32_t *name_lenp,
-				     int *poffsetp)
+				     uint32_t *poffsetp)
 {
 	const char *path, *name, *fixup_end;
 	const char *fixup_str = *valuep;
@@ -376,7 +376,7 @@ static int overlay_parse_fixup_entry(const char **valuep, int *lenp,
 	uint32_t fixup_len;
 	const char *sep;
 	char *endptr;
-	int poffset;
+	unsigned long poffset;
 
 	fixup_end = memchr(*valuep, '\0', *lenp);
 	if (!fixup_end)
@@ -388,7 +388,7 @@ static int overlay_parse_fixup_entry(const char **valuep, int *lenp,
 
 	path = fixup_str;
 	sep = memchr(fixup_str, ':', fixup_len);
-	if (!sep || *sep != ':')
+	if (!sep)
 		return -FDT_ERR_BADOVERLAY;
 
 	path_len = sep - path;
@@ -398,15 +398,17 @@ static int overlay_parse_fixup_entry(const char **valuep, int *lenp,
 	fixup_len -= path_len + 1;
 	name = sep + 1;
 	sep = memchr(name, ':', fixup_len);
-	if (!sep || *sep != ':')
+	if (!sep)
 		return -FDT_ERR_BADOVERLAY;
 
 	name_len = sep - name;
 	if (!name_len)
 		return -FDT_ERR_BADOVERLAY;
 
+	if ((*(sep + 1) < '0') || (*(sep + 1) > '9'))
+		return -FDT_ERR_BADOVERLAY;
 	poffset = strtoul(sep + 1, &endptr, 10);
-	if ((*endptr != '\0') || (endptr <= (sep + 1)))
+	if ((*endptr != '\0') || (poffset > (INT32_MAX - sizeof(uint32_t))))
 		return -FDT_ERR_BADOVERLAY;
 
 	*pathp = path;
@@ -470,8 +472,8 @@ static int overlay_fixup_phandle(void *fdt, void *fdto, int symbols_off,
 
 	do {
 		const char *path, *name;
-		uint32_t path_len, name_len;
-		int poffset, ret;
+		uint32_t path_len, name_len, poffset;
+		int ret;
 
 		ret = overlay_parse_fixup_entry(&value, &len, &path, &path_len,
 						&name, &name_len, &poffset);
@@ -925,8 +927,8 @@ static int get_path_len(const void *fdt, int nodeoffset)
 
 /**
  * overlay_split_fragment_path - Split "/<frag>/__overlay__[/<rel>]" into parts
- * @path: Input NUL-terminated path property string
- * @path_len: Length of @path in bytes (including trailing NUL)
+ * @path: Input path string (need not be NUL-terminated)
+ * @path_len: Length of @path in bytes (excluding any terminator)
  * @frag_namep: Output pointer to the fragment name
  * @frag_name_lenp: Output length of @frag_namep in bytes
  * @rel_pathp: Output pointer to the relative subnode path after __overlay__
@@ -935,7 +937,7 @@ static int get_path_len(const void *fdt, int nodeoffset)
  *
  * returns:
  *      0 on success
- *      -FDT_ERR_BADVALUE if @path is not a valid NUL-terminated '/' path
+ *      -FDT_ERR_BADVALUE if @path does not start with '/'
  */
 static int overlay_split_fragment_path(const char *path, int path_len,
 				       const char **frag_namep,
@@ -951,18 +953,14 @@ static int overlay_split_fragment_path(const char *path, int path_len,
 
 	*is_overlayp = false;
 
-	/* verify it's a string property (terminated by a single \0) */
-	if (path_len < 1 || memchr(path, '\0', path_len) != &path[path_len - 1])
-		return -FDT_ERR_BADVALUE;
-
 	/* keep end marker to avoid strlen() */
 	e = path + path_len;
 
-	if (*path != '/')
+	if (path_len < 1 || *path != '/')
 		return -FDT_ERR_BADVALUE;
 
 	/* get fragment name first */
-	s = strchr(path + 1, '/');
+	s = memchr(path + 1, '/', path_len - 1);
 	if (!s) {
 		/* Symbol refers to something that won't end
 		 * up in the target tree */
@@ -972,13 +970,13 @@ static int overlay_split_fragment_path(const char *path, int path_len,
 	frag_name = path + 1;
 	frag_name_len = s - path - 1;
 
-	/* verify format; safe since "s" lies in \0 terminated prop */
+	/* verify format */
 	len = sizeof("/__overlay__/") - 1;
-	if ((e - s) > len && (memcmp(s, "/__overlay__/", len) == 0)) {
+	if ((e - s) >= len && (memcmp(s, "/__overlay__/", len) == 0)) {
 		/* /<fragment-name>/__overlay__/<relative-subnode-path> */
 		rel_path = s + len;
-		rel_path_len = e - rel_path - 1;
-	} else if ((e - s) == len
+		rel_path_len = e - rel_path;
+	} else if ((e - s) == (len - 1)
 		   && (memcmp(s, "/__overlay__", len - 1) == 0)) {
 		/* /<fragment-name>/__overlay__ */
 		rel_path = "";
@@ -1049,7 +1047,11 @@ static int overlay_symbol_update(void *fdt, void *fdto)
 		if (!path)
 			return path_len;
 
-		ret = overlay_split_fragment_path(path, path_len,
+		/* verify it's a string property (terminated by a single \0) */
+		if (path_len < 1 || memchr(path, '\0', path_len) != &path[path_len - 1])
+			return -FDT_ERR_BADVALUE;
+
+		ret = overlay_split_fragment_path(path, path_len - 1,
 						  &frag_name, &frag_name_len,
 						  &rel_path, &rel_path_len,
 						  &is_overlay);
-- 
2.56.0.313.g2479322e68-goog