SUNDIALS init helpers assume fixed thread count and can index past allocated vectors

[WeiqunZhang]

• Type: Correctness / Bounds safety
• Severity: Medium
• Component: SUNDIALS context and memory-helper initialization
• Location:
  • Src/Extern/SUNDIALS/AMReX_Sundials_Core.cpp:18
  • Src/Extern/SUNDIALS/AMReX_Sundials_Core.cpp:24
  • Src/Extern/SUNDIALS/AMReX_SUNMemory.cpp:176
  • Src/Extern/SUNDIALS/AMReX_SUNMemory.cpp:182

Problem

Both Initialize(int nthreads) implementations resize internal vectors only when empty, then iterate i < nthreads and index initialized[i]/storage arrays directly.

If Initialize is called again with larger nthreads, indexing can go out of bounds.

Impact

• Out-of-bounds access if thread count changes across reinitialization.
• Latent crash/corruption in dynamic thread-count scenarios.

Suggested patch

Resize vectors when nthreads exceeds current size.

--- a/Src/Extern/SUNDIALS/AMReX_Sundials_Core.cpp
+++ b/Src/Extern/SUNDIALS/AMReX_Sundials_Core.cpp
@@
-    if (initialized.empty()) {
+    if (initialized.size() < static_cast<std::size_t>(nthreads)) {
+        auto old = initialized.size();
         initialized.resize(nthreads);
-        std::fill(initialized.begin(), initialized.end(), 0);
+        std::fill(initialized.begin()+old, initialized.end(), 0);
         the_sundials_context.resize(nthreads);
-        std::fill(the_sundials_context.begin(), the_sundials_context.end(), nullptr);
+        std::fill(the_sundials_context.begin()+old, the_sundials_context.end(), nullptr);
     }

Apply the same pattern in MemoryHelper::Initialize.

Prepared by Codex

[ajnonaka]

@balos1 do you understand what Codex is worried about here?

[balos1]

I don't think that this is a problem. Initialize is only meant to be called once. Right now it is called in amrex::Initialize which, I thought, could only be called once. So, I believe, the case Codex is worried about will never happen.

[WeiqunZhang]

It's not high priority. But we might still want to fix it in the future just in case.