In preparation for adding a __result_use_check annotation to copyin() and related functions, start checking for errors from copyout() in the mps(4) user command handler. This should make it easier to catch bugs. Reviewed by: imp, asomers MFC after: 1 month Differential Revision: https://reviews.freebsd.org/D43176 (cherry picked from commit bcf4a7c7ace21a01d10003de9c7692f0887526c1)
Depending on the card's firmware version, it may return different length responses for MPI2_FUNCTION_IOC_FACTS. But the first part of the response contains the length of the rest, so query it first to get the length and then use that to size the buffer for the full response. Also, correctly zero-initialize MPI2_IOC_FACTS_REQUEST. It only worked by luck before. PR: 264848 Reported by: Julien Cigar <julien@perdition.city> Sponsored by: Axcient Reviewed by: scottl, imp Differential Revision: https://reviews.freebsd.org/D38739 (cherry picked from commit 7d154c4dc64e61af7ca536c4e9927fa07c675a83)
Remove /^[\s*]*__FBSDID\("\$FreeBSD\$"\);?\s*\n/
Remove /^\s*\*\n \*\s+\$FreeBSD\$$\n/
In every mpr and mps ioctl that copies kernel data to userland, validate that the requested length does not exceed the size of the kernel's buffer. Note that all of these ioctls already required root access. MFC after: 2 weeks Sponsored by: Axcient Reviewed by: imp Differential Revision: https://reviews.freebsd.org/D38842
*_CFG_PAGE ioctl handlers in the mpr, mps, and mpt drivers allocated a buffer of a caller-specified size, but copied to it a fixed size header. Add checks that the size is at least the required minimum. Note that the device nodes are owned by root:operator with 0640 permissions so the ioctls are not available to unprivileged users. This change includes suggestions from scottl, markj and mav. Two of the mpt cases were reported by Lucas Leong (@_wmliang_) of Trend Micro Zero Day Initiative; scottl reported the third case in mpt. Same issue found in mpr and mps after discussion with imp. Reported by: Lucas Leong (@_wmliang_), Trend Micro Zero Day Initiative Reviewed by: imp, mav MFC after: 3 days Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D34692
According to Broadcom, mixing 64-bit SGEs with 32-bit chain entries can lead to IOC Fault code 0x40000d04. This fault code has been observed to suddenly increase on certain machines when the OCA firmware images are deployed. The hardware interprets all elements of a 64-bit SGE, even ones marked as 32-bit. Depending on the other bits, this will just work, but sometimes generate the above fault. Broadcom recommends this practice, and the Linux and NetBSD drivers follow it. Rework the chaining code to use MPI2_SGE_CHAIN64 instead of MPI2_SGE_CHAIN32. Adjust MPS_SGC_SIZE from 8 to 12 to match the size of the new structure. Flag the structure as being 64-bits now. Since MPS_SGE64_SIZE and MPS_SGC_SIZE are the same now, mps_push_sge could be simplified (after the same fashion of mpr). The different number of cases collapse to whether or not there's room for the segments and if not we need a chain, however these changes haven't been made yet as the current code handles those cases properly with the new defines. Made chain_busaddr 64-bits, even though we ask for all allocations to be below 4GB for this tag. Use it to set both parts of the CHAIN64 address rather than baking the 4GB assumption. Add asserts around the allocation to detect and BUSDMA bugs in allocation. Remove asserts and associated comment in mpi_pre_fw_download and mpi_pre_fw_upload. The code does not, it seems, depend on this invariant. The mpr driver has similar code, no asserts and also doesn't depend on this. Adjust comments to reflect the updated size. Sponsored by: Netflix Reviewed by: scottl, mav Differential Revision: https://reviews.freebsd.org/D34016
The device mapping table contains sc->max_devices entries, so only indices in [0, sc->max_devices) are valid. MFC after: 3 days Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D27964
Previously we copied in the request into a stack-allocated structure that could be smaller than the request size. Furthermore, we checked the request size only after doing the copyin. Fix this by allocating a buffer to hold the request, then copying the buffer's contents into a command descriptor. This is a bit heavy-handed but I expect the overhead will not be noticeable. The approach of coping the header in first is susceptible to TOCTOU problems. Reviewed by: imp Reported by: maxpl0it@protonmail.com MFC after: 3 days Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D27963
that can be extended, but also ensure compile-time type checking. Refactor common code out of arch-specific implementations. Move the mpr and mps drivers to this new API. The template type remains visible to the consumer so that it can be allocated on the stack, but should be considered opaque.