Wcohen/coverity202411 #287
Merged
Conversation
This file contains 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
wcohen
force-pushed
the
wcohen/coverity202411
branch
from
7063b54 to
b976c71
Compare
|
Your are correct. Shomehow the fixup for linux-infiniband.c wasn't in there. Just pushed a version with the fixup included to this pull request. |
It is possible in add_ib_counter() that one of the strdup() operations
fails and the other succeeds. In cases where the struct referencing
them is being free because of a failure one of the strdup() any
allocated memory should be freed also. This was noted noted in a
coverity scan of PAPI:
Error: RESOURCE_LEAK (CWE-772):
papi-7.2.0b1-build/papi-7.2.0b1/src/components/infiniband/linux-infiniband.c:332:5: alloc_fn: Storage is returned from allocation function "strdup".
papi-7.2.0b1-build/papi-7.2.0b1/src/components/infiniband/linux-infiniband.c:332:5: var_assign: Assigning: "new_cnt->ev_file_name" = storage returned from "strdup(file_name)".
papi-7.2.0b1-build/papi-7.2.0b1/src/components/infiniband/linux-infiniband.c:338:9: leaked_storage: Freeing "new_cnt" without freeing its pointer field "ev_file_name" leaks the storage that "ev_file_name" points to.
# 336| {
# 337| PAPIERROR("cannot allocate memory for counter internal fields");
# 338|-> papi_free(new_cnt);
# 339| return (0);
# 340| }
Error: RESOURCE_LEAK (CWE-772):
papi-7.2.0b1-build/papi-7.2.0b1/src/components/infiniband/linux-infiniband.c:331:5: alloc_fn: Storage is returned from allocation function "strdup".
papi-7.2.0b1-build/papi-7.2.0b1/src/components/infiniband/linux-infiniband.c:331:5: var_assign: Assigning: "new_cnt->ev_name" = storage returned from "strdup(name)".
papi-7.2.0b1-build/papi-7.2.0b1/src/components/infiniband/linux-infiniband.c:338:9: leaked_storage: Freeing "new_cnt" without freeing its pointer field "ev_name" leaks the storage that "ev_name" points to.
# 336| {
# 337| PAPIERROR("cannot allocate memory for counter internal fields");
# 338|-> papi_free(new_cnt);
# 339| return (0);
# 340| }
…ized Initializing variable type to a sane value (PAPI_MH_TYPE_EMPTY) to make sure that the coverity static analysis does not generate the following message: Error: UNINIT (CWE-457): papi-7.2.0b1-build/papi-7.2.0b1/src/components/sysdetect/linux_cpu_utils.c:561:5: var_decl: Declaring variable "type" without initializer. papi-7.2.0b1-build/papi-7.2.0b1/src/components/sysdetect/linux_cpu_utils.c:593:5: uninit_use: Using uninitialized value "type". # 591| } # 592| # 593|-> *value = type; # 594| # 595| return CPU_SUCCESS;
…read Added code to properly free memory for the following situation that Coverity pointed out: Error: RESOURCE_LEAK (CWE-772): [#def49] [important] papi-7.2.0b1-build/papi-7.2.0b1/src/threads.c:117:2: alloc_fn: Storage is returned from allocation function "malloc". papi-7.2.0b1-build/papi-7.2.0b1/src/threads.c:117:2: var_assign: Assigning: "thread->running_eventset" = storage returned from "malloc(8UL * (size_t)papi_num_components)". papi-7.2.0b1-build/papi-7.2.0b1/src/threads.c:134:4: leaked_storage: Freeing "thread" without freeing its pointer field "running_eventset" leaks the storage that "running_eventset" points to. # 132| papi_free( thread->context[i] ); # 133| papi_free( thread->context ); # 134|-> papi_free( thread ); # 135| return ( NULL ); # 136|
The local path_name variable in link_lmsensors_libraries() needs to
be PATH_MAX in size to avoid possible overruns and scribbling over
parts of the stack.
Error: OVERRUN (CWE-119): [#def22] [important]
papi-7.2.0b1-build/papi-7.2.0b1/src/components/lmsensors/linux-lmsensors.c:398:7: overrun-buffer-arg: Overrunning array "path_name" of 1024 bytes by passing it to a function which accesses it at byte offset 4095 using argument "4096UL". [Note: The source code implementation of the function has been overridden by a builtin model.]
# 396| // Step 3: Try the explicit install default.
# 397| if (dl1 == NULL && lmsensors_root != NULL) { // if root given, try it.
# 398|-> snprintf(path_name, PATH_MAX, "%s/lib64/libsensors.so", lmsensors_root); // PAPI Root check.
# 399| dl1 = dlopen(path_name, RTLD_NOW | RTLD_GLOBAL); // Try to open that path.
# 400| }
Error: OVERRUN (CWE-119): [#def23] [important]
papi-7.2.0b1-build/papi-7.2.0b1/src/components/lmsensors/linux-lmsensors.c:404:7: overrun-buffer-arg: Overrunning array "path_name" of 1024 bytes by passing it to a function which accesses it at byte offset 4095 using argument "4096UL". [Note: The source code implementation of the function has been overridden by a builtin model.]
# 402| // Step 4: Try another explicit install default.
# 403| if (dl1 == NULL && lmsensors_root != NULL) { // if root given, try it.
# 404|-> snprintf(path_name, PATH_MAX, "%s/lib/libsensors.so", lmsensors_root); // PAPI Root check.
# 405| dl1 = dlopen(path_name, RTLD_NOW | RTLD_GLOBAL); // Try to open that path.
# 406| }
wcohen
force-pushed
the
wcohen/coverity202411
branch
from
b976c71 to
ed919ac
Compare
Treece-Burgess
approved these changes
Nov 21, 2024
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.
Fixes for issues reported by coverity, redo of #266
Went through coverity scan of papi and produced patches to address some of the issue reported. PAPI with this PR's patches has been build and tests were run to check that things compiled and functions as expected. This incorporates the changes suggested in pull request #266.
Author Checklist
Why this PR exists. Reference all relevant information, including background, issues, test failures, etc
Commits are self contained and only do one thing
Commits have a header of the form:
module: short descriptionCommits have a body (whenever relevant) containing a detailed description of the addressed problem and its solution
The PR needs to pass all the tests