d/freebsd-nq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Don't directly dereference a user pointer in the VPD ioctl.

Browse Source 
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
This commit is contained in:
John Baldwin 2018-08-31 16:10:01 +00:00
parent 2ac6dfb0c2
commit 74aa2d49d6
1 changed files with 11 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
sys/dev/pci/pci_user.c
View File

@ -446,6 +446,14 @@ pci_conf_match(u_long cmd, struct pci_match_conf *matches, int num_matches,
}
}
/*
* 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 *lvio)
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 *lvio)
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 *lvio)
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"));