[rtems commit] libfdt: Safer access to strings section

Sebastian Huber sebh at rtems.org
Thu Jul 19 05:07:42 UTC 2018


Module:    rtems
Branch:    master
Commit:    72273b68e4e45b63768ef4574e481e81481ba133
Changeset: http://git.rtems.org/rtems/commit/?id=72273b68e4e45b63768ef4574e481e81481ba133

Author:    David Gibson <david at gibson.dropbear.id.au>
Date:      Tue Nov 14 22:45:56 2017 +1100

libfdt: Safer access to strings section

fdt_string() is used to retrieve strings from a DT blob's strings section.
It's rarely used directly, but is widely used internally.

However, it doesn't do any bounds checking, which means in the case of a
corrupted blob it could access bad memory, which libfdt is supposed to
avoid.

This write a safe alternative to fdt_string, fdt_get_string().  It checks
both that the given offset is within the string section and that the string
it points to is properly \0 terminated within the section.  It also returns
the string's length as a convenience (since it needs to determine to do the
checks anyway).

fdt_string() is rewritten in terms of fdt_get_string() for compatibility.

Most of the diff here is actually testing infrastructure.

Signed-off-by: David Gibson <david at gibson.dropbear.id.au>
Tested-by: Alexey Kardashevskiy <aik at ozlabs.ru>
Reviewed-by: Alexey Kardashevskiy <aik at ozlabs.ru>

---

 cpukit/dtc/libfdt/fdt_ro.c | 61 +++++++++++++++++++++++++++++++++++++++++++---
 cpukit/include/libfdt.h    | 18 +++++++++++++-
 2 files changed, 75 insertions(+), 4 deletions(-)

diff --git a/cpukit/dtc/libfdt/fdt_ro.c b/cpukit/dtc/libfdt/fdt_ro.c
index 4f4ef44..347aa7b 100644
--- a/cpukit/dtc/libfdt/fdt_ro.c
+++ b/cpukit/dtc/libfdt/fdt_ro.c
@@ -76,17 +76,72 @@ static int fdt_nodename_eq_(const void *fdt, int offset,
 		return 0;
 }
 
+const char *fdt_get_string(const void *fdt, int stroffset, int *lenp)
+{
+	uint32_t absoffset = stroffset + fdt_off_dt_strings(fdt);
+	size_t len;
+	int err;
+	const char *s, *n;
+
+	err = fdt_ro_probe_(fdt);
+	if (err != 0)
+		goto fail;
+
+	err = -FDT_ERR_BADOFFSET;
+	if (absoffset >= fdt_totalsize(fdt))
+		goto fail;
+	len = fdt_totalsize(fdt) - absoffset;
+
+	if (fdt_magic(fdt) == FDT_MAGIC) {
+		if (stroffset < 0)
+			goto fail;
+		if (fdt_version(fdt) >= 17) {
+			if (stroffset >= fdt_size_dt_strings(fdt))
+				goto fail;
+			if ((fdt_size_dt_strings(fdt) - stroffset) < len)
+				len = fdt_size_dt_strings(fdt) - stroffset;
+		}
+	} else if (fdt_magic(fdt) == FDT_SW_MAGIC) {
+		if ((stroffset >= 0)
+		    || (stroffset < -fdt_size_dt_strings(fdt)))
+			goto fail;
+		if ((-stroffset) < len)
+			len = -stroffset;
+	} else {
+		err = -FDT_ERR_INTERNAL;
+		goto fail;
+	}
+
+	s = (const char *)fdt + absoffset;
+	n = memchr(s, '\0', len);
+	if (!n) {
+		/* missing terminating NULL */
+		err = -FDT_ERR_TRUNCATED;
+		goto fail;
+	}
+
+	if (lenp)
+		*lenp = n - s;
+	return s;
+
+fail:
+	if (lenp)
+		*lenp = err;
+	return NULL;
+}
+
 const char *fdt_string(const void *fdt, int stroffset)
 {
-	return (const char *)fdt + fdt_off_dt_strings(fdt) + stroffset;
+	return fdt_get_string(fdt, stroffset, NULL);
 }
 
 static int fdt_string_eq_(const void *fdt, int stroffset,
 			  const char *s, int len)
 {
-	const char *p = fdt_string(fdt, stroffset);
+	int slen;
+	const char *p = fdt_get_string(fdt, stroffset, &slen);
 
-	return (strlen(p) == len) && (memcmp(p, s, len) == 0);
+	return p && (slen == len) && (memcmp(p, s, len) == 0);
 }
 
 uint32_t fdt_get_max_phandle(const void *fdt)
diff --git a/cpukit/include/libfdt.h b/cpukit/include/libfdt.h
index 837c5e2..b50af54 100644
--- a/cpukit/include/libfdt.h
+++ b/cpukit/include/libfdt.h
@@ -288,6 +288,22 @@ int fdt_move(const void *fdt, void *buf, int bufsize);
 /**********************************************************************/
 
 /**
+ * fdt_get_string - retrieve a string from the strings block of a device tree
+ * @fdt: pointer to the device tree blob
+ * @stroffset: offset of the string within the strings block (native endian)
+ * @lenp: optional pointer to return the string's length
+ *
+ * fdt_get_string() retrieves a pointer to a single string from the
+ * strings block of the device tree blob at fdt, and optionally also
+ * returns the string's length in *lenp.
+ *
+ * returns:
+ *     a pointer to the string, on success
+ *     NULL, if stroffset is out of bounds, or doesn't point to a valid string
+ */
+const char *fdt_get_string(const void *fdt, int stroffset, int *lenp);
+
+/**
  * fdt_string - retrieve a string from the strings block of a device tree
  * @fdt: pointer to the device tree blob
  * @stroffset: offset of the string within the strings block (native endian)
@@ -297,7 +313,7 @@ int fdt_move(const void *fdt, void *buf, int bufsize);
  *
  * returns:
  *     a pointer to the string, on success
- *     NULL, if stroffset is out of bounds
+ *     NULL, if stroffset is out of bounds, or doesn't point to a valid string
  */
 const char *fdt_string(const void *fdt, int stroffset);
 



More information about the vc mailing list