From f447448aa3c6c944a27c29793b92f4d7fb5df432 Mon Sep 17 00:00:00 2001 From: David Robillard Date: Sun, 6 Oct 2024 18:25:33 -0400 Subject: Handle realloc failure and avoid potential null pointer arithmetic --- .suppress.cppcheck | 2 -- src/AtomForge.cpp | 21 ++++++++++++++++----- src/server/.clang-tidy | 1 - src/server/Buffer.cpp | 7 ++++++- src/server/ingen_lv2.cpp | 8 +++++++- 5 files changed, 29 insertions(+), 10 deletions(-) diff --git a/.suppress.cppcheck b/.suppress.cppcheck index 44919591..798a9c7c 100644 --- a/.suppress.cppcheck +++ b/.suppress.cppcheck @@ -7,11 +7,9 @@ cstyleCast duplInheritedMember duplicateExpression knownConditionTrueFalse -memleakOnRealloc missingReturn noExplicitConstructor normalCheckLevelMaxBranches -nullPointerArithmetic shadowFunction unsafeClassCanLeak useStlAlgorithm diff --git a/src/AtomForge.cpp b/src/AtomForge.cpp index 50747f33..ed54142a 100644 --- a/src/AtomForge.cpp +++ b/src/AtomForge.cpp @@ -25,6 +25,7 @@ #include #include +#include namespace ingen { @@ -73,11 +74,21 @@ AtomForge::append(const void* const data, const uint32_t len) // Update size and reallocate if necessary if (lv2_atom_pad_size(_size + len) > _capacity) { - _capacity = lv2_atom_pad_size(_size + len); - - _buf = - AtomPtr{static_cast(realloc(_buf.release(), _capacity)), - FreeDeleter{}}; + const size_t new_size = lv2_atom_pad_size(_size + len); + + // Release buffer and try to realloc it + auto* const old = _buf.release(); + auto* const new_buf = static_cast(realloc(old, new_size)); + if (!new_buf) { + // Failure, reclaim old buffer and signal error to caller + _buf = AtomPtr{old, FreeDeleter{}}; + return 0; + } + + // Adopt new buffer and update capacity to make room for the new data + std::unique_ptr> ptr{new_buf}; + _capacity = new_size; + _buf = std::move(ptr); } // Append new data diff --git a/src/server/.clang-tidy b/src/server/.clang-tidy index 6990b1fe..a580cc7e 100644 --- a/src/server/.clang-tidy +++ b/src/server/.clang-tidy @@ -6,7 +6,6 @@ Checks: > -bugprone-branch-clone, -bugprone-parent-virtual-call, -bugprone-reserved-identifier, - -bugprone-suspicious-realloc-usage, -bugprone-suspicious-string-compare, -cert-dcl37-c, -cert-dcl51-cpp, diff --git a/src/server/Buffer.cpp b/src/server/Buffer.cpp index ea3205fb..68b75931 100644 --- a/src/server/Buffer.cpp +++ b/src/server/Buffer.cpp @@ -172,7 +172,12 @@ void Buffer::resize(uint32_t capacity) { if (!_external) { - _buf = realloc(_buf, capacity); + void* const new_buf = realloc(_buf, capacity); + if (!new_buf) { + throw std::bad_alloc{}; + } + + _buf = new_buf; _capacity = capacity; clear(); } else { diff --git a/src/server/ingen_lv2.cpp b/src/server/ingen_lv2.cpp index 930f350d..76bdcb54 100644 --- a/src/server/ingen_lv2.cpp +++ b/src/server/ingen_lv2.cpp @@ -318,7 +318,13 @@ public: break; } - buf = realloc(buf, sizeof(LV2_Atom) + atom.size); + void* const new_buf = realloc(buf, sizeof(LV2_Atom) + atom.size); + if (!new_buf) { + _engine.log().rt_error("Failed to allocate for from-UI ring\n"); + break; + } + + buf = new_buf; memcpy(buf, &atom, sizeof(LV2_Atom)); if (!_from_ui.read(atom.size, -- cgit v1.2.1