Fix/memory resource safety#627
Open
gfnord wants to merge 4 commits into
Open
Conversation
The 255-byte char buffer used by CUpdateCheck was allocated with raw new[] and freed only after several std::string operations that can throw bad_alloc on OOM. Switched to std::unique_ptr<char[]> so the buffer cannot leak on the exception path. No behavioural change in the success path; the same 254-byte GitHub-API read still happens through the same WinINet handles.
Two fixes in LFX_Initialize. NODEVS path: when no devices are detected, LFX_Initialize previously returned LFX_ERROR_NODEVS leaving the worker thread, its two events, and afx_dev allocated for process lifetime. Now the NODEVS branch stops the worker (SetEvent + WaitForSingleObject + CloseHandle on the thread and both events) and then deletes afx_dev. CLightsProc at LightFX.cpp:54 dereferences afx_dev with no NULL guard, so the thread MUST be stopped before afx_dev is freed; the new code does it in that order and a comment documents the invariant so future cleanup passes don't reorder it back into a use-after-free. CreateThread failure path: if CreateThread inside the same block returned NULL, the two CreateEvent handles created moments earlier leaked and afx_dev was left orphaned. The new if (!updateThread) branch closes both events, NULLs them, frees afx_dev, and returns LFX_ERROR_NOINIT.
DrawFreq() already returns the window DC via ReleaseDC(hysto, hdc_r), so the following DeleteDC(hdc_r) was a no-op on an already-released handle (DeleteDC is for CreateCompatibleDC results, not GetDC results). Removed the redundant line.
Two fixes in WSAudioIn.cpp. GetDefaultMultimediaDevice() declared IMMDeviceEnumerator* pEnumerator uninitialized then tested it with if (pEnumerator) after CoCreateInstance. If CoCreateInstance failed the out-param was not guaranteed NULL and the check read indeterminate stack memory. Now initialized to NULL. FFTProc() allocated kiss_cfg + three new[] arrays without rollback; a bad_alloc in any later allocation leaked the earlier ones, and kiss_fftr_alloc returning NULL caused a NULL dereference in kiss_fftr below. Switched the three arrays to std::unique_ptr<T[]>, moved kiss_fftr_alloc after them, and added a NULL check that returns early. Updated the kiss_fftr call site to use .get(). Removed the three matching delete[] lines at function exit; kiss_fft_free is retained.
gfnord
added a commit
to gfnord/alienfx-tools
that referenced
this pull request
Jun 25, 2026
Memory- and resource-safety fixes (see PR T-Troll#627): - Common/Common.cpp: RAII buffer in CUpdateCheck - LightFX/LightFX.cpp: LFX_Initialize cleanup on NODEVS + CreateThread failure - alienfx-gui/HapticsDialog.cpp: drop redundant DeleteDC - alienfx-gui/WSAudioIn.cpp: NULL-init COM enumerator, RAII FFT buffers
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A set of memory- and resource-safety fixes across four files. No behavioural change on the success path; each change closes a leak, a use-after-free risk, or an uninitialized read on a failure/exception path.
Changes
Common/Common.cpp— Update check buffer exception safetyThe 255-byte
charbuffer used byCUpdateCheckwas allocated with rawnew[]and freed only after severalstd::stringoperations that can throwbad_allocon OOM. Switched tostd::unique_ptr<char[]>so the buffer cannot leak on the exception path. The 254-byte GitHub-API read through the same WinINet handles is unchanged.LightFX/LightFX.cpp— Init resource cleanup on failure pathsTwo fixes in
LFX_Initialize:NODEVSpath: when no devices are detected, the previous code returnedLFX_ERROR_NODEVSleaving the worker thread, its two events, andafx_devallocated for process lifetime. Now theNODEVSbranch stops the worker (SetEvent+WaitForSingleObject+CloseHandleon the thread and both events) and then deletesafx_dev.CLightsProcatLightFX.cpp:54dereferencesafx_devwith no NULL guard, so the thread is stopped beforeafx_devis freed, and a comment documents the invariant so future cleanup passes do not reorder it into a use-after-free.CreateThreadfailure path: ifCreateThreadin the same block returnedNULL, the twoCreateEventhandles created moments earlier leaked andafx_devwas left orphaned. The newif (!updateThread)branch closes both events, NULLs them, freesafx_dev, and returnsLFX_ERROR_NOINIT.alienfx-gui/HapticsDialog.cpp— Drop redundantDeleteDCDrawFreq()already returns the window DC viaReleaseDC(hysto, hdc_r), so the followingDeleteDC(hdc_r)was a no-op on an already-released handle (DeleteDCis forCreateCompatibleDCresults, notGetDCresults). Removed the redundant line.alienfx-gui/WSAudioIn.cpp— Initialize COM enumerator, RAII FFT buffersTwo fixes:
GetDefaultMultimediaDevice()declaredIMMDeviceEnumerator* pEnumeratoruninitialized, then tested it withif (pEnumerator)afterCoCreateInstance. IfCoCreateInstancefailed, the out-param was not guaranteedNULLand the check read indeterminate stack memory. Now initialized toNULL.FFTProc()allocatedkiss_cfg+ threenew[]arrays without rollback; abad_allocin any later allocation leaked the earlier ones, andkiss_fftr_allocreturningNULLcaused aNULLdereference inkiss_fftrbelow. Switched the three arrays tostd::unique_ptr<T[]>, movedkiss_fftr_allocafter them, and added aNULLcheck that returns early. Updated thekiss_fftrcall site to use.get(). Removed the three matchingdelete[]lines at function exit;kiss_fft_freeis retained.Notes