Skip to content

Commit 2c5ecf0

Browse files
janherlingmetameta-codesync[bot]
authored andcommitted
Ocean: Reject a zero maximal payload transfer size instead of dividing by it
Summary: `determineIsochronousTransferLayout()` divides by `endpointPacketSize` at line 581, reached whenever `endpointPacketSize >= maxPayloadTransferSize`. `endpointPacketSize` starts at 0 and stays 0 for an altsetting with no endpoints - which is exactly the zero-bandwidth altsetting 0 of a UVC streaming interface, and that altsetting must exist, because the caller only takes this path when the interface has more than one altsetting. So with `maxPayloadTransferSize == 0` the condition reads `0 >= 0`, the gate opens and the division runs with a zero divisor. The `ocean_assert(endpointPacketSize >= 1u)` directly above is compiled out in release builds. The consequence is architecture dependent. On x86 the division raises `#DE` and the process takes SIGFPE. On AArch64 `udiv` by zero does not trap, it produces 0, so the function silently returns altsetting 0 with `transferSize`, `packetsPerTransfer` and `bytesPerPacket` all zero, and the caller then submits degenerate zero-length isochronous transfers. Two changes. `determineIsochronousTransferLayout()` returns -1 immediately for a zero payload size, since no altsetting can satisfy a transfer of that size and the only reason the loop appeared to match one was the same zero. A separate `endpointPacketSize != 0u` guard is not needed on top: once the payload size is at least 1, `endpointPacketSize >= maxPayloadTransferSize` already implies the divisor is non-zero. And `VideoDevice::start()` now aborts on a zero `dwMaxPayloadTransferSize` rather than only logging. That check sat between two neighbours which both release the subscription and return, and was the odd one out in forwarding the value anyway. It also covers the bulk path, which the early return in `Device` does not reach and which otherwise builds a zero-length transfer. landed-with-radar-review Reviewed By: enpe Differential Revision: D115344588 fbshipit-source-id: d36d1644beabbda112a19ab87f44093b8ec39aa8
1 parent b53e32a commit 2c5ecf0

2 files changed

Lines changed: 9 additions & 0 deletions

File tree

impl/ocean/system/usb/Device.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -531,6 +531,12 @@ int Device::determineIsochronousTransferLayout(libusb_context* usbContext, const
531531
ocean_assert(maxVideoFrameSize != 0u);
532532
ocean_assert(maxPayloadTransferSize != 0u);
533533

534+
if (maxPayloadTransferSize == 0u)
535+
{
536+
// every altsetting would satisfy the packet size check below, including a zero-bandwidth altsetting without any endpoint, which would then divide by zero
537+
return -1;
538+
}
539+
534540
transferSize = 0;
535541

536542
for (int altsettingIndex = 0; altsettingIndex < interface.num_altsetting; ++altsettingIndex)

impl/ocean/system/usb/video/VideoDevice.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1912,6 +1912,9 @@ bool VideoDevice::start(const unsigned int preferredWidth, const unsigned int pr
19121912
if (dwMaxPayloadTransferSize == 0u)
19131913
{
19141914
Log::error() << "Unknown maximal payload size";
1915+
1916+
claimedVideoStreamInterfaceSubscription_.release();
1917+
return false;
19151918
}
19161919

19171920
if (activeClockFrequency_ == 0u)

0 commit comments

Comments
 (0)