Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 30 Oct 2018 21:31:33 +0000 (UTC)
From:      John Baldwin <jhb@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org
Subject:   svn commit: r339932 - in stable: 10/sys/dev/pci 11/sys/dev/pci
Message-ID:  <201810302131.w9ULVXjk005796@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: jhb
Date: Tue Oct 30 21:31:32 2018
New Revision: 339932
URL: https://svnweb.freebsd.org/changeset/base/339932

Log:
  MFC 338408: Don't directly dereference a user pointer in the VPD ioctl.
  
  The PCIOCLISTVPD ioctl on /dev/pci is used to fetch a list of VPD
  key-value pairs for a specific PCI function.  It is used by
  'pciconf -l -V'.  The list is stored in a userland-supplied buffer as
  an array of variable-length structures where the key and data length
  are stored in a fixed-size header followed by the variable-length
  value as a byte array.  To facilitate walking this array in userland,
  <sys/pciio.h> provides a PVE_NEXT() helper macro to return a pointer
  to the next array element by reading the the length out of the current
  header and using it to compute the address of the next header.
  
  To simplify the implementation, the ioctl handler was also using
  PVE_NEXT() when on the user address of the user buffer to compute the
  user address of the next array element.  However, the PVE_NEXT() macro
  when used with a user address was reading the value's length by
  indirecting the user pointer.  The value was ready after the current
  record had been copied out to the user buffer, so it appeared to work
  on architectures where user addresses are directly dereferencable from
  the kernel (all but powerpc and i386 after the 4:4 split).  The recent
  enablement of SMAP on amd64 caught this violation however.  To fix,
  add a variant of PVE_NEXT() for use in the ioctl handler that takes an
  explicit value length.

Modified:
  stable/11/sys/dev/pci/pci_user.c
Directory Properties:
  stable/11/   (props changed)

Changes in other areas also in this revision:
Modified:
  stable/10/sys/dev/pci/pci_user.c
Directory Properties:
  stable/10/   (props changed)

Modified: stable/11/sys/dev/pci/pci_user.c
==============================================================================
--- stable/11/sys/dev/pci/pci_user.c	Tue Oct 30 20:51:03 2018	(r339931)
+++ stable/11/sys/dev/pci/pci_user.c	Tue Oct 30 21:31:32 2018	(r339932)
@@ -406,6 +406,14 @@ pci_conf_match_old32(struct pci_match_conf_old32 *matc
 #endif	/* COMPAT_FREEBSD32 */
 #endif	/* PRE7_COMPAT */
 
+/*
+ * Like PVE_NEXT but takes an explicit length since 'pve' is a user
+ * pointer that cannot be dereferenced.
+ */
+#define	PVE_NEXT_LEN(pve, datalen)					\
+	((struct pci_vpd_element *)((char *)(pve) +			\
+	    sizeof(struct pci_vpd_element) + (datalen)))
+
 static int
 pci_list_vpd(device_t dev, struct pci_list_vpd_io *lvio)
 {
@@ -454,7 +462,7 @@ pci_list_vpd(device_t dev, struct pci_list_vpd_io *lvi
 	    strlen(vpd->vpd_ident));
 	if (error)
 		return (error);
-	vpd_user = PVE_NEXT(vpd_user);
+	vpd_user = PVE_NEXT_LEN(vpd_user, vpd_element.pve_datalen);
 	vpd_element.pve_flags = 0;
 	for (i = 0; i < vpd->vpd_rocnt; i++) {
 		vpd_element.pve_keyword[0] = vpd->vpd_ros[i].keyword[0];
@@ -467,7 +475,7 @@ pci_list_vpd(device_t dev, struct pci_list_vpd_io *lvi
 		    vpd->vpd_ros[i].len);
 		if (error)
 			return (error);
-		vpd_user = PVE_NEXT(vpd_user);
+		vpd_user = PVE_NEXT_LEN(vpd_user, vpd_element.pve_datalen);
 	}
 	vpd_element.pve_flags = PVE_FLAG_RW;
 	for (i = 0; i < vpd->vpd_wcnt; i++) {
@@ -481,7 +489,7 @@ pci_list_vpd(device_t dev, struct pci_list_vpd_io *lvi
 		    vpd->vpd_w[i].len);
 		if (error)
 			return (error);
-		vpd_user = PVE_NEXT(vpd_user);
+		vpd_user = PVE_NEXT_LEN(vpd_user, vpd_element.pve_datalen);
 	}
 	KASSERT((char *)vpd_user - (char *)lvio->plvi_data == len,
 	    ("length mismatch"));



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201810302131.w9ULVXjk005796>