Merge pull request #15034 from MarcZX10/fix/p19-usbd-transfer-bounds

fix(libSceUsbd): bound ISO packet counts and transfer buffers

core/libs/prx/libSceUsbd/Export.cpp

@@ -53,6 +53,7 @@ struct UsbdState {
     std::unordered_map<libusb_device_handle*, std::uint32_t> claimed;
     std::unordered_set<libusb_config_descriptor*> configs;
     std::unordered_map<UsbdTransfer*, libusb_transfer*> inFlight;
+    std::unordered_map<const UsbdTransfer*, std::int32_t> isoCapacities;
 };
 
 UsbdState& State() {
@@ -76,6 +77,11 @@ std::int32_t ToGuestError(int result) {
     return static_cast<std::int32_t>(0x80240000u - static_cast<std::uint32_t>(result));
 }
 
+std::int32_t IsoCapacity(const UsbdState& state, const UsbdTransfer* transfer) {
+    const auto found = state.isoCapacities.find(transfer);
+    return found == state.isoCapacities.end() ? 0 : found->second;
+}
+
 bool IsKnownDevice(const UsbdState& state, libusb_device* device) {
     return state.devices.contains(device) || std::any_of(state.handles.begin(), state.handles.end(), [device](libusb_device_handle* handle) { return libusb_get_device(handle) == device; });
 }
@@ -341,6 +347,9 @@ UsbdTransfer* APS5_VABI sceUsbdAllocTransfer(std::int32_t isoPackets) {
     auto* transfer = static_cast<UsbdTransfer*>(std::calloc(1, size));
     if (transfer == nullptr) return nullptr;
     transfer->numIsoPackets = isoPackets;
+    auto& state = State();
+    std::lock_guard lock(state.mutex);
+    state.isoCapacities[transfer] = isoPackets;
     return transfer;
 }
 
@@ -350,6 +359,7 @@ void APS5_VABI sceUsbdFreeTransfer(UsbdTransfer* transfer) {
         auto& state = State();
         std::lock_guard lock(state.mutex);
         if (state.inFlight.contains(transfer)) throw std::runtime_error(std::string(__func__) + ": transfer is still in flight");
+        state.isoCapacities.erase(transfer);
     }
     std::free(transfer);
 }
@@ -380,6 +390,11 @@ void APS5_VABI sceUsbdFillBulkTransfer(UsbdTransfer* transfer, void* deviceHandl
 
 void APS5_VABI sceUsbdFillIsoTransfer(UsbdTransfer* transfer, void* deviceHandle, std::uint8_t endpoint, std::uint8_t* buffer, std::int32_t length, std::int32_t numIsoPackets, UsbdTransferCallback callback, void* userData, std::uint32_t timeout) {
     if (transfer == nullptr) throw std::runtime_error(std::string(__func__) + ": null transfer");
+    {
+        auto& state = State();
+        std::lock_guard lock(state.mutex);
+        if (numIsoPackets > IsoCapacity(state, transfer)) throw std::runtime_error(std::string(__func__) + ": the iso packet count is larger than the transfer was allocated for");
+    }
     transfer->deviceHandle = deviceHandle;
     transfer->endpoint = endpoint;
     transfer->type = TRANSFER_TYPE_ISOCHRONOUS;
@@ -427,15 +442,27 @@ std::uint8_t* APS5_VABI sceUsbdControlTransferGetSetup(UsbdTransfer* transfer) {
 
 void APS5_VABI sceUsbdSetIsoPacketLengths(UsbdTransfer* transfer, std::uint32_t length) {
     if (transfer == nullptr) throw std::runtime_error(std::string(__func__) + ": null transfer");
+    {
+        auto& state = State();
+        std::lock_guard lock(state.mutex);
+        if (transfer->numIsoPackets > IsoCapacity(state, transfer)) throw std::runtime_error(std::string(__func__) + ": the iso packet count is larger than the transfer was allocated for");
+    }
     for (std::int32_t packet = 0; packet < transfer->numIsoPackets; ++packet) transfer->isoPacketDescriptors[packet].length = length;
 }
 
 std::uint8_t* APS5_VABI sceUsbdGetIsoPacketBuffer(UsbdTransfer* transfer, std::uint32_t packet) {
     if (transfer == nullptr) throw std::runtime_error(std::string(__func__) + ": null transfer");
-    if (transfer->numIsoPackets < 0 || packet >= static_cast<std::uint32_t>(transfer->numIsoPackets) || transfer->buffer == nullptr) return nullptr;
-    std::uint8_t* pointer = transfer->buffer;
-    for (std::uint32_t index = 0; index < packet; ++index) pointer += transfer->isoPacketDescriptors[index].length;
-    return pointer;
+    std::int32_t capacity = 0;
+    {
+        auto& state = State();
+        std::lock_guard lock(state.mutex);
+        capacity = IsoCapacity(state, transfer);
+    }
+    if (transfer->numIsoPackets < 0 || transfer->numIsoPackets > capacity || packet >= static_cast<std::uint32_t>(transfer->numIsoPackets) || transfer->buffer == nullptr) return nullptr;
+    std::uint64_t offset = 0;
+    for (std::uint32_t index = 0; index < packet; ++index) offset += transfer->isoPacketDescriptors[index].length;
+    if (transfer->length < 0 || offset + transfer->isoPacketDescriptors[packet].length > static_cast<std::uint64_t>(transfer->length)) return nullptr;
+    return transfer->buffer + offset;
 }
 
 std::int32_t APS5_VABI sceUsbdBulkTransfer(void* deviceHandle, std::uint8_t endpoint, std::uint8_t* data, std::int32_t length, std::int32_t* transferred, std::uint32_t timeout) {
@@ -618,6 +645,12 @@ std::int32_t APS5_VABI sceUsbdSetConfiguration(void* deviceHandle, std::int32_t
 
 std::int32_t APS5_VABI sceUsbdSubmitTransfer(UsbdTransfer* transfer) {
     if (transfer == nullptr || transfer->deviceHandle == nullptr) return SCE_USBD_ERROR_INVALID_ARG;
+    auto& state = State();
+    {
+        std::lock_guard lock(state.mutex);
+        if (transfer->numIsoPackets > IsoCapacity(state, transfer)) return SCE_USBD_ERROR_INVALID_ARG;
+    }
+    if (transfer->length < 0 || (transfer->length > 0 && transfer->buffer == nullptr)) return SCE_USBD_ERROR_INVALID_ARG;
     auto* handle = KnownHandle(transfer->deviceHandle, __func__);
     if ((transfer->flags & ~SUPPORTED_TRANSFER_FLAGS) != 0) throw std::runtime_error(std::string(__func__) + ": transfer flags " + std::to_string(transfer->flags) + " are not supported");
     if (transfer->numIsoPackets < 0) return SCE_USBD_ERROR_INVALID_ARG;
@@ -634,7 +667,6 @@ std::int32_t APS5_VABI sceUsbdSubmitTransfer(UsbdTransfer* transfer) {
     host->callback = CompleteTransfer;
     host->num_iso_packets = transfer->numIsoPackets;
     for (int packet = 0; packet < transfer->numIsoPackets; ++packet) host->iso_packet_desc[packet].length = transfer->isoPacketDescriptors[packet].length;
-    auto& state = State();
     {
         std::lock_guard lock(state.mutex);
         if (!state.inFlight.emplace(transfer, host).second) {

core/libs/tests/GuestUsbd.cpp

@@ -426,6 +426,9 @@ void CheckTransferFillers() {
     for (int packet = 0; packet < 3; ++packet) Require(transfer->isoPacketDescriptors[packet].length == 16, "iso packet length not set");
     Require(sceUsbdGetIsoPacketBuffer(transfer, 0) == data && sceUsbdGetIsoPacketBuffer(transfer, 1) == data + 16 && sceUsbdGetIsoPacketBuffer(transfer, 2) == data + 32, "iso packet buffers do not follow the packet lengths");
     Require(sceUsbdGetIsoPacketBuffer(transfer, 3) == nullptr, "a packet past the last one has a buffer");
+    sceUsbdSetIsoPacketLengths(transfer, 17);
+    Require(sceUsbdGetIsoPacketBuffer(transfer, 2) == nullptr, "a packet that runs past the transfer buffer has a buffer");
+    sceUsbdSetIsoPacketLengths(transfer, 16);
     std::uint8_t control[12] = {};
     sceUsbdFillControlSetup(control, 0x80, 6, 0x0100, 0x0409, 4);
     Require(control[0] == 0x80 && control[1] == 6 && control[2] == 0x00 && control[3] == 0x01 && control[4] == 0x09 && control[5] == 0x04 && control[6] == 4 && control[7] == 0, "control setup not packed little-endian");
@@ -437,6 +440,23 @@ void CheckTransferFillers() {
     Require(Throws([] { sceUsbdFillBulkTransfer(nullptr, nullptr, 0, nullptr, 0, nullptr, nullptr, 0); }), "filling a null bulk transfer did not throw");
     Require(Throws([] { sceUsbdFillControlSetup(nullptr, 0, 0, 0, 0, 0); }), "filling a null control setup did not throw");
     Require(Throws([] { sceUsbdGetIsoPacketBuffer(nullptr, 0); }), "the packet buffer of a null transfer did not throw");
+    auto* bulk = sceUsbdAllocTransfer(0);
+    Require(bulk != nullptr, "transfer allocation failed");
+    sceUsbdFillBulkTransfer(bulk, &device, 0x02, nullptr, 16, &MarkDone, &user, 0);
+    Require(sceUsbdSubmitTransfer(bulk) == invalidArgument, "a submit with a null buffer and a length was accepted");
+    sceUsbdFillBulkTransfer(bulk, &device, 0x02, data, -1, &MarkDone, &user, 0);
+    Require(sceUsbdSubmitTransfer(bulk) == invalidArgument, "a submit with a negative length was accepted");
+    sceUsbdFreeTransfer(bulk);
+    Require(Throws([&] { sceUsbdFillIsoTransfer(transfer, &device, 0x83, data, 64, 4, &MarkDone, &user, 0); }), "an iso fill beyond the allocated packets did not throw");
+    auto* small = sceUsbdAllocTransfer(1);
+    Require(small != nullptr, "transfer allocation failed");
+    Require(Throws([&] { sceUsbdFillIsoTransfer(small, &device, 0x83, data, 64, 2, &MarkDone, &user, 0); }), "an iso fill beyond a one packet transfer did not throw");
+    sceUsbdFillIsoTransfer(small, &device, 0x83, data, 16, 1, &MarkDone, &user, 0);
+    small->numIsoPackets = 2;
+    Require(sceUsbdGetIsoPacketBuffer(small, 0) == nullptr, "the packet buffer of a count above the allocation was returned");
+    Require(sceUsbdSubmitTransfer(small) == invalidArgument, "a submit above the allocated packet count was accepted");
+    Require(Throws([&] { sceUsbdSetIsoPacketLengths(small, 8); }), "setting iso packet lengths above the allocated packet count did not throw");
+    sceUsbdFreeTransfer(small);
     sceUsbdFreeTransfer(transfer);
 }
 

docs/dev/debt/libSceUsbd.md

@@ -6,6 +6,7 @@
 - [libSceUsbd errors](../../../core/libs/prx/libSceUsbd/Export.cpp) (libSceUsbd) - errors are libusb's codes mapped to `0x80240000 - code`, with `LIBUSB_ERROR_OTHER` at `0x802400FF`, following [shadPS4](https://github.com/shadps4-emu/shadPS4/blob/main/src/core/libraries/usbd/usbd.cpp); the null arguments FreeBSD libusb rejects ([libusb10.c](https://github.com/freebsd/freebsd-src/blob/d106002967eff712894ab8b5337bbbff106ce5e0/lib/libusb/libusb10.c), [libusb10_desc.c](https://github.com/freebsd/freebsd-src/blob/d106002967eff712894ab8b5337bbbff106ce5e0/lib/libusb/libusb10_desc.c), [libusb10_io.c](https://github.com/freebsd/freebsd-src/blob/d106002967eff712894ab8b5337bbbff106ce5e0/lib/libusb/libusb10_io.c)) return `0x80240002`, and `sceUsbdOpen` and `sceUsbdGetActiveConfigDescriptor` throw on a null device, which it dereferences.
 - [libSceUsbd transfers, kernel drivers and host access](../../../core/libs/prx/libSceUsbd/Export.cpp) (libSceUsbd) - a submitted transfer is copied into a host transfer whose completion copies the status, actual length and iso packet results back and then calls the title's callback from event handling; transfer flags other than short-not-ok and add-zero-packet throw. `sceUsbdAttachKernelDriver` returns 0 as FreeBSD's stub does, `sceUsbdCheckConnected` reports a device gone when reading its configuration fails with `LIBUSB_ERROR_NO_DEVICE` (FreeBSD asks the kernel directly). When the host has no USB access, initialization keeps the empty device list and event handling waits for its timeout. Opening, descriptors, string and control requests, interface claims and an asynchronous control transfer were checked against an Xbox 360 controller on Windows; the transfer layout, the argument lists, the error codes and isochronous transfers are unverified on PS5
 
+- [sceUsbdFillIsoTransfer, sceUsbdSetIsoPacketLengths, sceUsbdSubmitTransfer, sceUsbdGetIsoPacketBuffer](../../../core/libs/prx/libSceUsbd/Export.cpp) (libSceUsbd) - host memory-safety bounds, not taken from FreeBSD 11.4 [libusb10.c](https://github.com/freebsd/freebsd-src/blob/d106002967eff712894ab8b5337bbbff106ce5e0/lib/libusb/libusb10.c) and not measured on a console: `sceUsbdFillIsoTransfer` and `sceUsbdSetIsoPacketLengths` throw for an iso packet count above the one the transfer was allocated for; `sceUsbdSubmitTransfer` returns `SCE_USBD_ERROR_INVALID_ARG` for that count, for a null buffer with a non-zero length and for a negative length; `sceUsbdGetIsoPacketBuffer` returns null for a count above the allocation and for a packet that does not end inside the transfer length. Closes when: measured on a PS5
 ### Functional
 
 - [sceUsbdGetConfiguration, sceUsbdGetConfigDescriptorByValue, sceUsbdGetDescriptor, sceUsbdGetStringDescriptorAscii](../../../core/libs/prx/libSceUsbd/Export.cpp) (libSceUsbd) - not measured on a console; they follow FreeBSD 11.4 ([libusb10_desc.c](https://github.com/freebsd/freebsd-src/blob/d106002967eff712894ab8b5337bbbff106ce5e0/lib/libusb/libusb10_desc.c), [libusb20.c](https://github.com/freebsd/freebsd-src/blob/d106002967eff712894ab8b5337bbbff106ce5e0/lib/libusb/libusb20.c)). The configuration and descriptor queries return host libusb errors. `sceUsbdGetStringDescriptorAscii` rebuilds `libusb20_dev_req_string_simple_sync` with only its 255-byte read: libusb's Windows HID backend reports the requested length as `bLength`, cutting a 4-byte header read to one character. Unlike the host `libusb_get_string_descriptor_ascii` it accepts index 0, drops non-printable characters and `<>` instead of writing `?`, and fails with `0x802400FF`. Closes when: measured on a PS5