Page MenuHomeFreeBSD

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

Authored by jhb on Aug 19 2018, 2:00 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Oct 20, 7:20 PM
Unknown Object (File)
Mon, Sep 30, 5:44 AM
Unknown Object (File)
Sep 18 2024, 6:43 PM
Unknown Object (File)
Sep 18 2024, 9:43 AM
Unknown Object (File)
Sep 17 2024, 2:36 PM
Unknown Object (File)
Sep 12 2024, 12:02 PM
Unknown Object (File)
Sep 10 2024, 10:23 PM
Unknown Object (File)
Sep 10 2024, 12:54 AM

Details

Summary

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

Test Plan

I haven't tested or compiled this yet. Am hoping to get test feedback from Jeff next week.

Score one for SMAP! (I wonder if brooks@ already ran into this in CheriBSD or not.)

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

PVE_NEXT() becomes unused in the kernel after the change. Might be move the macro to usr.sbin/pciconf for its only use ? Or change PVE_NEXT() to take the length.

I had considered PVE_NEXT part of the API for userland kind of like CMSG_NEXT() (or whatever that is called) in case other binaries want to use the ioctl. (I don't know of any that do off-hand, I just considered it part of the API.)

I do not think PVE_NEXT() is a (useful) API.

This revision is now accepted and ready to land.Aug 19 2018, 5:00 PM
This revision was automatically updated to reflect the committed changes.