svn commit: r334414 - in head/sys: kern sys

[Warner Losh]

Author: imp
Date: Thu May 31 02:57:58 2018
New Revision: 334414
URL: https://svnweb.freebsd.org/changeset/base/334414

Log:
  Make the data returned by devinfo harder to overflow.
  
  Rather than using fixed-length strings, pack them into a string table
  to return. Also expand the buffer from ~300 charaters to 3k. This should
  be enough, even for USB.
  
  This fixes a problem where USB pnp info is truncated on return to
  userland.
  
  Differential Revision: https://reviews.freebsd.org/D15629

Modified:
  head/sys/kern/subr_bus.c
  head/sys/sys/bus.h

Modified: head/sys/kern/subr_bus.c
==============================================================================
--- head/sys/kern/subr_bus.c	Thu May 31 02:57:51 2018	(r334413)
+++ head/sys/kern/subr_bus.c	Thu May 31 02:57:58 2018	(r334414)
@@ -5264,8 +5264,9 @@ sysctl_devices(SYSCTL_HANDLER_ARGS)
 	u_int			namelen = arg2;
 	int			index;
 	device_t		dev;
-	struct u_device		udev;	/* XXX this is a bit big */
+	struct u_device		*udev;
 	int			error;
+	char			*walker, *ep;
 
 	if (namelen != 2)
 		return (EINVAL);
@@ -5286,24 +5287,45 @@ sysctl_devices(SYSCTL_HANDLER_ARGS)
 		return (ENOENT);
 
 	/*
-	 * Populate the return array.
+	 * Populate the return item, careful not to overflow the buffer.
 	 */
-	bzero(&udev, sizeof(udev));
-	udev.dv_handle = (uintptr_t)dev;
-	udev.dv_parent = (uintptr_t)dev->parent;
-	if (dev->nameunit != NULL)
-		strlcpy(udev.dv_name, dev->nameunit, sizeof(udev.dv_name));
-	if (dev->desc != NULL)
-		strlcpy(udev.dv_desc, dev->desc, sizeof(udev.dv_desc));
-	if (dev->driver != NULL && dev->driver->name != NULL)
-		strlcpy(udev.dv_drivername, dev->driver->name,
-		    sizeof(udev.dv_drivername));
-	bus_child_pnpinfo_str(dev, udev.dv_pnpinfo, sizeof(udev.dv_pnpinfo));
-	bus_child_location_str(dev, udev.dv_location, sizeof(udev.dv_location));
-	udev.dv_devflags = dev->devflags;
-	udev.dv_flags = dev->flags;
-	udev.dv_state = dev->state;
-	error = SYSCTL_OUT(req, &udev, sizeof(udev));
+	udev = malloc(sizeof(*udev), M_BUS, M_WAITOK | M_ZERO);
+	if (udev == NULL)
+		return (ENOMEM);
+	udev->dv_handle = (uintptr_t)dev;
+	udev->dv_parent = (uintptr_t)dev->parent;
+	udev->dv_devflags = dev->devflags;
+	udev->dv_flags = dev->flags;
+	udev->dv_state = dev->state;
+	walker = udev->dv_fields;
+	ep = walker + sizeof(udev->dv_fields);
+#define CP(src)						\
+	if ((src) == NULL)				\
+		*walker++ = '\0';			\
+	else {						\
+		strlcpy(walker, (src), ep - walker);	\
+		walker += strlen(walker) + 1;		\
+	}						\
+	if (walker >= ep)				\
+		break;
+
+	do {
+		CP(dev->nameunit);
+		CP(dev->desc);
+		CP(dev->driver != NULL ? dev->driver->name : NULL);
+		bus_child_pnpinfo_str(dev, walker, ep - walker);
+		walker += strlen(walker) + 1;
+		if (walker >= ep)
+			break;
+		bus_child_location_str(dev, walker, ep - walker);
+		walker += strlen(walker) + 1;
+		if (walker >= ep)
+			break;
+		*walker++ = '\0';
+	} while (0);
+#undef CP
+	error = SYSCTL_OUT(req, udev, sizeof(*udev));
+	free(udev, M_BUS);
 	return (error);
 }
 

Modified: head/sys/sys/bus.h
==============================================================================
--- head/sys/sys/bus.h	Thu May 31 02:57:51 2018	(r334413)
+++ head/sys/sys/bus.h	Thu May 31 02:57:58 2018	(r334414)
@@ -46,7 +46,7 @@
  */
 struct u_businfo {
 	int	ub_version;		/**< @brief interface version */
-#define BUS_USER_VERSION	1
+#define BUS_USER_VERSION	2
 	int	ub_generation;		/**< @brief generation count */
 };
 
@@ -63,20 +63,23 @@ typedef enum device_state {
 
 /**
  * @brief Device information exported to userspace.
+ * The strings are placed one after the other, separated by NUL characters.
+ * Fields should be added after the last one and order maintained for compatibility
  */
+#define BUS_USER_BUFFER		(3*1024)
 struct u_device {
 	uintptr_t	dv_handle;
 	uintptr_t	dv_parent;
-
-	char		dv_name[32];		/**< @brief Name of device in tree. */
-	char		dv_desc[32];		/**< @brief Driver description */
-	char		dv_drivername[32];	/**< @brief Driver name */
-	char		dv_pnpinfo[128];	/**< @brief Plug and play info */
-	char		dv_location[128];	/**< @brief Where is the device? */
 	uint32_t	dv_devflags;		/**< @brief API Flags for device */
 	uint16_t	dv_flags;		/**< @brief flags for dev state */
 	device_state_t	dv_state;		/**< @brief State of attachment */
-	/* XXX more driver info? */
+	char		dv_fields[BUS_USER_BUFFER]; /**< @brief NUL terminated fields */
+	/* name (name of the device in tree) */
+	/* desc (driver description) */
+	/* drivername (Name of driver without unit number) */
+	/* pnpinfo (Plug and play information from bus) */
+	/* location (Location of device on parent */
+	/* NUL */
 };
 
 /* Flags exported via dv_flags. */