Fix FTDI GetDeviceInfoList stack overflow + duplicate serials (auto-config)#61
Fix FTDI GetDeviceInfoList stack overflow + duplicate serials (auto-config)#61dynajoe wants to merge 1 commit into
Conversation
|
DirectOutput - Post-Fix.log @dynajoe - the good news, if i change the Cabinet.xml AutoConfigEnabled to true VPX no longer crashes. the bad news, dof doesn't work at all. In the Pre-Fix, with AutoConfigEnabled to false, the LEDs work. in the Post-Fix log file you can see the serial numbers are not being resolved with the dll build from your fix branch. let me know if i can supply anything else. |
|
Hello. Thanks for the PR! Converting this to draft as per the contributing rules. (I was behind in getting them added as was just done in Visual Pinball). It's going to take time to review these. libdof is currently is use by several users with a variety of devices, and we have yet to receive issues about this (with the exception of @emb417 specific Window's issue) Also this definitely needs thorough testing with the actual hardware. |
…-config FT245RBitbangControllerAutoConfigurator passed a single-element stack node to GetDeviceInfoList once per device. GetDeviceInfoList writes one node per detected device, so with 2+ FTDI boards it wrote past the 1-element buffer (a std::string assignment into out-of-bounds stack) -> memory corruption/crash; and the loop index i was never used, so every controller got device[0] serial/description (duplicate serials, only the first board addressable). Fill one correctly-sized buffer in a single call and index by i, matching the C# OpenByIndex(i) loop. Also make GetDeviceInfoList honor the callers buffer capacity (like the real FTD2XX API) so an undersized buffer cannot overflow again.
913bb6e to
f80342b
Compare
|
@dynajoe, thanks for another round of patching. I grabbed the latest from the fix/branch and built the dll and pdb. i updated my config to auto config true, loaded a table, and inspected the directoutput.log which looks the exactly same with new timestamps. The description and serial number for both ftdi devices are blank. and the leds stop working. essentially, with these code changes i'm observing zero change in behavior. |
3 participants
Fix
FT245RBitbangControllerAutoConfiguratorcalledGetDeviceInfoListwith a single-element stack node, once per device.GetDeviceInfoListwrites one node per detected device, so:std::stringassignment into out-of-bounds stack memory) → memory corruption / crash;iwas never used to select a device, so every controller got device[0]'s serial/description (duplicate serials; only the first board addressable).Fill one correctly-sized buffer in a single call and index by
i, matching the C#OpenByIndex(i)loop. Also makeGetDeviceInfoListhonor the caller's buffer capacity (like the real FTD2XX API), so an undersized buffer can't overflow it again —GetDeviceInfoListhas a single caller, so this is contained to the FTDI path.Why it matters
Addresses the crash + wrong-serial half of #40. (The "no toys" half — the LedWizEquivalent resolution — is #60.) The overflow only triggers with 2+ FT245R boards, so it's multi-board setups (e.g. Sainsmart) that crash.
I don't have FTDI hardware, so this is verified against the DirectOutput C# source and by reasoning — not bench-tested. I'd rather not have it merged on a code read alone.
@emb417 — you have the Sainsmart FT245R setup and the crash reported in #40. Would you be able to build this branch and confirm: (a) it no longer crashes with your boards attached, and (b) each controller now logs its own serial number (rather than all showing device 0's)? Happy to adjust if it behaves differently than the C# leads me to expect.