Date: Tue, 30 Oct 2018 21:31:32 +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-10@freebsd.org Subject: svn commit: r339932 - in stable: 10/sys/dev/pci 11/sys/dev/pci Message-ID: <201810302131.w9ULVWlT005790@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/10/sys/dev/pci/pci_user.c Directory Properties: stable/10/ (props changed) Changes in other areas also in this revision: Modified: stable/11/sys/dev/pci/pci_user.c Directory Properties: stable/11/ (props changed) Modified: stable/10/sys/dev/pci/pci_user.c ============================================================================== --- stable/10/sys/dev/pci/pci_user.c Tue Oct 30 20:51:03 2018 (r339931) +++ stable/10/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.w9ULVWlT005790>