fix(POSIX port): check pthread_key_create/pthread_setspecific return values#1453
Open
saikumar-mandaji wants to merge 1 commit into
Open
Conversation
…values prvInitThreadKey() ignored pthread_key_create()'s return value, and prvMarkAsFreeRTOSThread() ignored pthread_setspecific()'s return value. If pthread_setspecific() fails, the one-byte thread marker allocated just before it is neither stored in TLS (so prvThreadKeyDestructor() never runs for it, leaking the allocation) nor freed locally -- and the thread is left unmarked, so prvIsFreeRTOSThread() silently misclassifies it as not a FreeRTOS thread. The correctness impact (thread identity) is larger than the one-byte leak itself. Fix: check both return values. On pthread_setspecific() failure, free the marker to avoid the leak, then call prvFatalError() -- the same handling this port already uses for other unexpected pthread call failures (pthread_create(), sigaction()). This function returns void, so there is no way to propagate a failure to the caller; continuing past a failed pthread_setspecific() would leave the thread silently misclassified rather than failing loudly. Same treatment for pthread_key_create(), since a failure there means every later pthread_setspecific()/pthread_getspecific() call on xThreadKey is operating on an invalid key. Fixes FreeRTOS#1447 Signed-off-by: Saikumar Mandaji <mandajisaikumar@gmail.com>
|
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
2 participants
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.
Fixes #1447
Problem
prvInitThreadKey()ignoredpthread_key_create()'s return value, andprvMarkAsFreeRTOSThread()ignoredpthread_setspecific()'s return value.If
pthread_setspecific()fails, the one-byte thread marker allocated just before it is neither stored in TLS (soprvThreadKeyDestructor()never runs for it, leaking the allocation) nor freed locally -- and the thread is left unmarked, soprvIsFreeRTOSThread()silently misclassifies it as not a FreeRTOS thread. The correctness impact (thread identity) is larger than the one-byte leak itself.Fix
Check both return values. On
pthread_setspecific()failure, free the marker to avoid the leak, then callprvFatalError()-- the same handling this port already uses for other unexpected pthread call failures (pthread_create(),sigaction()). This function returnsvoid, so there is no way to propagate a failure to the caller; continuing past a failedpthread_setspecific()would leave the thread silently misclassified rather than failing loudly.Same treatment for
pthread_key_create(), since a failure there means every laterpthread_setspecific()/pthread_getspecific()call onxThreadKeyis operating on an invalid key.Verification
Verified by hand against the issue's analysis and by matching the existing
prvFatalError()usage pattern already present in this same file forpthread_create()/sigaction()failures. No C compiler was available in the environment this patch was authored in, so I was not able to build/run the POSIX port's test suite directly -- please double-check against CI/local build before merging.