Skip site navigation (1)Skip section navigation (2) 
Date:      Fri, 31 Aug 2018 16:10:01 +0000 (UTC)
From:      John Baldwin <jhb@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r338408 - head/sys/dev/pci
Message-ID:  <201808311610.w7VGA1cR081427@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: jhb
Date: Fri Aug 31 16:10:01 2018
New Revision: 338408
URL: https://svnweb.freebsd.org/changeset/base/338408

Log:
  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.
  
  Reported by:	Jeffrey Pieper @ Intel
  Reviewed by:	kib
  Approved by:	re (gjb)
  MFC after:	1 week
  Differential Revision:	https://reviews.freebsd.org/D16800

Modified:
  head/sys/dev/pci/pci_user.c

Modified: head/sys/dev/pci/pci_user.c
==============================================================================
--- head/sys/dev/pci/pci_user.c	Fri Aug 31 15:02:53 2018	(r338407)
+++ head/sys/dev/pci/pci_user.c	Fri Aug 31 16:10:01 2018	(r338408)
@@ -446,6 +446,14 @@ pci_conf_match(u_long cmd, struct pci_match_conf *matc
 	}
 }
 
+/*
+ * 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)
 {
@@ -494,7 +502,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];
@@ -507,7 +515,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++) {
@@ -521,7 +529,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?201808311610.w7VGA1cR081427>