svn commit: r367643 - in head: sys/dev/ofw usr.sbin/ofwdump

[Brandon Bergren]

Author: bdragon
Date: Fri Nov 13 16:49:41 2020
New Revision: 367643
URL: https://svnweb.freebsd.org/changeset/base/367643

Log:
  [PowerPC] Allow traversal of oversize OF properties.
  
  In standards such as LoPAPR, property names in excess of the usual 31
  characters exist.
  
  This breaks property traversal.
  
  While in IEEE 1275-1994, nextprop is defined explicitly to work with a
  32-byte region of memory, using a larger buffer should be fine. There is
  actually no way to pass a buffer length to the nextprop call in the OF
  client interface, so SLOF actually just blindly overflows the buffer.
  
  So we have to defensively make the buffer larger, to avoid memory
  corruption when reading out long properties on live OF systems.
  
  Note also that on real-mode OF, things are pretty tight because we are
  allocating against a static bounce buffer in low memory, so we can't just
  use a huge buffer to work around this without it being wasteful of our
  limited amount of 32-bit physical memory.
  
  This allows a patched ofwdump to operate properly on SLOF (i.e. pseries)
  systems, as well as any other PowerPC systems with overlength properties.
  
  Reviewed by:	jhibbits
  MFC after:	2 weeks
  Sponsored by:	Tag1 Consulting, Inc.
  Differential Revision:	https://reviews.freebsd.org/D26669

Modified:
  head/sys/dev/ofw/openfirmio.c
  head/sys/dev/ofw/openfirmio.h
  head/usr.sbin/ofwdump/ofwdump.c

Modified: head/sys/dev/ofw/openfirmio.c
==============================================================================
--- head/sys/dev/ofw/openfirmio.c	Fri Nov 13 16:47:42 2020	(r367642)
+++ head/sys/dev/ofw/openfirmio.c	Fri Nov 13 16:49:41 2020	(r367643)
@@ -115,7 +115,7 @@ openfirm_ioctl(struct cdev *dev, u_long cmd, caddr_t d
 	phandle_t node;
 	int len, ok, error;
 	char *name, *value;
-	char newname[32];
+	char newname[OFIOCSUGGPROPNAMELEN];
 
 	if ((flags & FREAD) == 0)
 		return (EBADF);
@@ -222,8 +222,19 @@ openfirm_ioctl(struct cdev *dev, u_long cmd, caddr_t d
 			break;
 		}
 		len = strlen(newname) + 1;
-		if (len > of->of_buflen)
+		if (len > of->of_buflen) {
+			/*
+			 * Passed buffer was insufficient.
+			 *
+			 * Instead of returning an error here, truncate the
+			 * property name to fit the buffer.
+			 *
+			 * This allows us to retain compatibility with old
+			 * tools which always pass a 32 character buffer.
+			 */
 			len = of->of_buflen;
+			newname[len - 1] = '\0';
+		}
 		else
 			of->of_buflen = len;
 		error = copyout(newname, of->of_buf, len);

Modified: head/sys/dev/ofw/openfirmio.h
==============================================================================
--- head/sys/dev/ofw/openfirmio.h	Fri Nov 13 16:47:42 2020	(r367642)
+++ head/sys/dev/ofw/openfirmio.h	Fri Nov 13 16:49:41 2020	(r367643)
@@ -76,4 +76,18 @@ struct ofiocdesc {
 /* Maximum accepted value length (maximum of nvramrc property). */
 #define	OFIOCMAXVALUE	8192
 
+/*
+ * While IEEE 1275-1994 states in 3.2.2.1.1 that property names are 1-31
+ * printable characters, in practice, this limit has been ignored.
+ * Noncompliant properties have been codified in standards such as LoPAPR.
+ *
+ * This is a suggested buffer length that should be large enough to hold
+ * any property name currently seen in device trees, without being overly
+ * wasteful of memory.
+ *
+ * If a future version of the Devicetree specification updates the property
+ * names length requirement, this value will be updated to match.
+ */
+#define	OFIOCSUGGPROPNAMELEN	64
+
 #endif /* _DEV_OFW_OPENFIRMIO_H_ */

Modified: head/usr.sbin/ofwdump/ofwdump.c
==============================================================================
--- head/usr.sbin/ofwdump/ofwdump.c	Fri Nov 13 16:47:42 2020	(r367642)
+++ head/usr.sbin/ofwdump/ofwdump.c	Fri Nov 13 16:49:41 2020	(r367643)
@@ -144,7 +144,7 @@ static void
 ofw_dump_properties(int fd, phandle_t n, int level, int raw, int str)
 {
 	int nlen;
-	char prop[32];
+	char prop[OFIOCSUGGPROPNAMELEN];
 
 	for (nlen = ofw_firstprop(fd, n, prop, sizeof(prop)); nlen != 0;
 	     nlen = ofw_nextprop(fd, n, prop, prop, sizeof(prop)))