Fix coverity issues - #22835
Merged
Merged
Conversation
…ion on numerical edge cases. - Simplify `snprintfz` logic by removing redundant null checks in cgroup cache stat handling. - Add null check in stream receiver logging to safeguard against early handshake failures.
Contributor
There was a problem hiding this comment.
1 issue found across 3 files
Confidence score: 2/5
- In
src/ml/ml.cc, the catch path does not resetdim->training_in_progressunderslock, so any exception can leave that dimension permanently stuck as “training,” blocking all future training attempts for that dimension after merge — add the same state cleanup used by other early-return paths in the catch block before merging.
Architecture diagram
sequenceDiagram
participant SR as Stream Receiver
participant Log as Log System
participant ML as ML Training Worker
participant Dlib as Dlib KMeans
participant CG as Cgroup Ebpf Cachestat
participant Proc as /proc Filesystem
Note over SR,Proc: Runtime interaction flows for Coverity fixes
SR->>SR: stream_receiver_remove_internal()
alt rpt->host is NULL (early handshake failure)
SR->>Log: ND_LOG_FIELD_STR(NDF_NIDL_NODE, NULL)
Note over SR,Log: CHANGED: Guarded dereference - passes NULL instead of crashing
else rpt->host exists
SR->>Log: ND_LOG_FIELD_STR(NDF_NIDL_NODE, rpt->host->hostname)
end
SR->>Log: ND_LOG_FIELD_TXT(NDF_SRC_IP, rpt->remote_ip)
SR->>Log: ND_LOG_FIELD_TXT(NDF_SRC_PORT, rpt->remote_port)
Note over ML,Dlib: ML training dimension flow
ML->>ML: ml_dimension_train_model()
ML->>Dlib: ml_kmeans_init()
ML->>Dlib: ml_kmeans_train()
alt Training succeeds
Dlib-->>ML: model trained
ML->>ML: update models
else Dlib throws exception (numerical edge case)
Dlib-->>ML: std::exception (e.g. dlib::fatal_error)
Note over ML: CHANGED: Catch exception, log error, skip model for round
ML->>ML: return ML_WORKER_RESULT_NOT_ENOUGH_COLLECTED_VALUES
end
Note over CG,Proc: Cgroup ebpf cachestat flow
CG->>CG: cgroup_ebpfgo_open_nonempty_procs_file(path_buf, ...)
Note over CG: path_buf is always a stack array (caller guarantee)
CG->>Proc: try to open /proc files for cgroup
alt Non-empty proc file found
Proc-->>CG: procfile read
CG->>CG: snprintfz(best_path, ..., path_buf)
Note over CG: CHANGED: Removed redundant null checks on path_buf
CG->>CG: snprintfz(path_buf, ..., best_path)
else Empty or no file
Proc-->>CG: null or empty
CG->>CG: continue iteration
end
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
stelfrag
marked this pull request as ready for review
June 29, 2026 21:12
thiagoftsm
approved these changes
Jun 30, 2026
thiagoftsm
left a comment
Contributor
There was a problem hiding this comment.
PR is working as expected. LGTM!
|
stelfrag
added a commit
to stelfrag/netdata
that referenced
this pull request
Jul 12, 2026
Backport of netdata#22835 to 2.10.x, excluding the cgroup_ebpfgo_cachestat.c snprintf hunk (that file does not exist in this branch). - ml.cc: guard ml_kmeans_train() with try/catch so a dlib exception on numerical edge cases skips the dimension instead of terminating netdata. - stream-receiver.c: null-check rpt->host in the log stack on early handshake failure.
Merged
Ferroin
pushed a commit
that referenced
this pull request
Jul 15, 2026
Backport of #22835 to 2.10.x, excluding the cgroup_ebpfgo_cachestat.c snprintf hunk (that file does not exist in this branch). - ml.cc: guard ml_kmeans_train() with try/catch so a dlib exception on numerical edge cases skips the dimension instead of terminating netdata. - stream-receiver.c: null-check rpt->host in the log stack on early handshake failure.
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.
Summary
Summary by cubic
Fixes three Coverity findings to improve stability: guards a potential null dereference in stream receiver logs, treats ML KMeans numerical errors as transient to avoid aborts and stuck state, and removes redundant checks in cgroup cache stat.
rpt->hostin disconnect logs to avoid null deref on early handshake failures.path_bufnull checks; caller always passes a stack buffer.dlib::errorfromml_kmeans_train, log it, cleartraining_in_progress, and skip the model for this round; let non-dlib exceptions propagate.Written for commit dc8034e. Summary will update on new commits.