Skip to content

fix: strtok mutates c_str() via const cast - #14

Open
andrewwhitecdw wants to merge 1 commit into
NVIDIA:mainfrom
andrewwhitecdw:codequality/fmpm-strtok-mutates-c-str-via-const-cast
Open

fix: strtok mutates c_str() via const cast#14
andrewwhitecdw wants to merge 1 commit into
NVIDIA:mainfrom
andrewwhitecdw:codequality/fmpm-strtok-mutates-c-str-via-const-cast

Conversation

@andrewwhitecdw

@andrewwhitecdw andrewwhitecdw commented Jul 31, 2026

Copy link
Copy Markdown

fix: avoid UB when strtok mutates c_str() via const cast

Summary

parsePartitionIdlistString() in fmpm.cpp tokenizes a comma-separated partition list with strtok, but passes the pointer returned by std::string::c_str() after casting away its const qualifier. Because strtok writes into its first argument, this is undefined behavior and can lead to silent corruption or crashes depending on the implementation of std::string.

Root cause

std::string::c_str() returns const char *:

char * token = strtok((char *)partitionListStr.c_str(), ",");

strtok mutates the input buffer to insert null terminators between tokens. Casting away const does not change the fact that the underlying storage was obtained as read-only through c_str(), so any write through that pointer is UB. Most implementations happen to make it work today, but it is not guaranteed and can break with different compilers, -D_GLIBCXX_DEBUG, or custom allocators.

Fix

Use &partitionListStr[0] to obtain a mutable pointer to the string's contiguous internal buffer (contiguity and mutability are guaranteed for std::string since C++11):

-    char * token = strtok((char *)partitionListStr.c_str(), ",");
+    char * token = strtok(&partitionListStr[0], ",");

This preserves the existing parsing behavior while removing the undefined const cast. An empty input string still causes strtok to return NULL on the first call and the loop exits with *numPartitions = 0, matching the original observable behavior.

Testing

  • Verified that the change compiles cleanly.
  • I did not add a new regression test because this repository does not appear to have a C++ unit-test harness for fmpm.cpp. If maintainers would like a test added, I am happy to add one in whatever form the project prefers.

Why existing tests missed it

This is a C-level undefined-behavior issue in a utility function; it would not be caught by build or lint checks unless a sanitizer/UBSan run is executed against this code path, which does not appear to be part of CI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant