Conversation
Passing NULL to SetConfigValue for k_ESteamNetworkingConfig_ConnectionUserData at connection scope took the user data special case, which converted the value with AssignConfigValueTyped() and dereferenced the NULL pointer. The check in SetConfigValueTyped() that rejects clearing connection user data was never reached. Let NULL fall through to SetConfigValueTyped(), so the call fails with "Cannot clear connection user data" instead of crashing, and add a check for it to the pipe test.
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
1 participant
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.
The header says you can pass NULL to SetConfigValue to clear a setting that isn't global. For
k_ESteamNetworkingConfig_ConnectionUserDataat connection scope, that crashes. The user data special case inCSteamNetworkingUtils::SetConfigValuerunsAssignConfigValueTyped( &newData, eDataType, pValue )before anything checks for NULL, so it dereferences the null pointer.SetConfigValueTyped()already has a branch that rejects this ("Cannot clear connection user data"). Both were added in ab5ef56, but that branch is never reached for connection scope.The fix lets a NULL value fall through to
SetConfigValueTyped(). The call now returns false with that error instead of crashing. Setting a non-NULL value still goes through the special case, same as before.To verify it, I added a check to the
pipetest intest_connectionthat tries to clear the user data on a socket pair connection. On master,test_connection pipecrashes with an access violation (0xC0000005) at that call. With this change it passes and logs "Cannot clear connection user data".test_connection suite-quickalso passes. I ran everything on Windows with MSVC 2022.