From 3e8aeeb7d6cb58811b7ec5be57f576aef7855e13 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Damian=20Kr=C3=B3lik?=
 <66667989+Damian-Nordic@users.noreply.github.com>
Date: Thu, 21 Dec 2023 17:06:02 +0100
Subject: [PATCH] [app] Fix DeferredAttributePersister memory leak (#31075)

* [app] Fix DeferredAttributePerister memory leak

ScopedMemoryBuffer's Release() method was used instead of
Free(). Add CHECK_RETURN_VALUE annotation to the Release()
method to prevent from making such a mistake in the future.

Signed-off-by: Damian Krolik <damian.krolik@nordicsemi.no>

* Code review

---------

Signed-off-by: Damian Krolik <damian.krolik@nordicsemi.no>
---
 .../DeferredAttributePersistenceProvider.cpp  |  2 +-
 src/darwin/Framework/CHIP/MTRBaseDevice.mm    |  4 +--
 src/lib/support/ScopedBuffer.h                | 30 +++++++++++++------
 3 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/src/app/DeferredAttributePersistenceProvider.cpp b/src/app/DeferredAttributePersistenceProvider.cpp
index 62d5c446fe2091..82652792806d5e 100644
--- a/src/app/DeferredAttributePersistenceProvider.cpp
+++ b/src/app/DeferredAttributePersistenceProvider.cpp
@@ -39,7 +39,7 @@ void DeferredAttribute::Flush(AttributePersistenceProvider & persister)
 {
     VerifyOrReturn(IsArmed());
     persister.WriteValue(mPath, ByteSpan(mValue.Get(), mValue.AllocatedSize()));
-    mValue.Release();
+    mValue.Free();
 }
 
 CHIP_ERROR DeferredAttributePersistenceProvider::WriteValue(const ConcreteAttributePath & aPath, const ByteSpan & aValue)
diff --git a/src/darwin/Framework/CHIP/MTRBaseDevice.mm b/src/darwin/Framework/CHIP/MTRBaseDevice.mm
index a4bcf3742c8d6e..09a8e07b0e34da 100644
--- a/src/darwin/Framework/CHIP/MTRBaseDevice.mm
+++ b/src/darwin/Framework/CHIP/MTRBaseDevice.mm
@@ -1142,8 +1142,8 @@ - (void)readAttributePaths:(NSArray<MTRAttributeRequestPath *> * _Nullable)attri
             //
             callback->AdoptReadClient(std::move(readClient));
             callback.release();
-            attributePathParamsList.Release();
-            eventPathParamsList.Release();
+            IgnoreUnusedVariable(attributePathParamsList.Release());
+            IgnoreUnusedVariable(eventPathParamsList.Release());
             return err;
         });
     std::move(*bridge).DispatchAction(self);
diff --git a/src/lib/support/ScopedBuffer.h b/src/lib/support/ScopedBuffer.h
index 258b7f262935fb..7a3c0aaca5902c 100644
--- a/src/lib/support/ScopedBuffer.h
+++ b/src/lib/support/ScopedBuffer.h
@@ -25,6 +25,7 @@
 #pragma once
 
 #include <lib/support/CHIPMem.h>
+#include <lib/support/CodeUtils.h>
 
 #include <type_traits>
 #include <utility>
@@ -84,10 +85,11 @@ class ScopedMemoryBufferBase
     const void * Ptr() const { return mBuffer; }
 
     /**
-     * Releases the undelying buffer. Buffer stops being managed and will not be
-     * auto-freed.
+     * Releases the underlying buffer.
+     *
+     * The buffer stops being managed and will not be auto-freed.
      */
-    void * Release()
+    CHECK_RETURN_VALUE void * Release()
     {
         void * buffer = mBuffer;
         mBuffer       = nullptr;
@@ -139,13 +141,18 @@ class ScopedMemoryBuffer : public Impl::ScopedMemoryBufferBase<MemoryManagement>
 
     static_assert(std::is_trivially_destructible<T>::value, "Destructors won't get run");
 
-    inline T * Get() { return static_cast<T *>(Base::Ptr()); }
-    inline T & operator[](size_t index) { return Get()[index]; }
+    T * Get() { return static_cast<T *>(Base::Ptr()); }
+    T & operator[](size_t index) { return Get()[index]; }
 
-    inline const T * Get() const { return static_cast<const T *>(Base::Ptr()); }
-    inline const T & operator[](size_t index) const { return Get()[index]; }
+    const T * Get() const { return static_cast<const T *>(Base::Ptr()); }
+    const T & operator[](size_t index) const { return Get()[index]; }
 
-    inline T * Release() { return static_cast<T *>(Base::Release()); }
+    /**
+     * Releases the underlying buffer.
+     *
+     * The buffer stops being managed and will not be auto-freed.
+     */
+    CHECK_RETURN_VALUE T * Release() { return static_cast<T *>(Base::Release()); }
 
     ScopedMemoryBuffer & Calloc(size_t elementCount)
     {
@@ -222,7 +229,12 @@ class ScopedMemoryBufferWithSize : public ScopedMemoryBuffer<T>
         ScopedMemoryBuffer<T>::Free();
     }
 
-    T * Release()
+    /**
+     * Releases the underlying buffer.
+     *
+     * The buffer stops being managed and will not be auto-freed.
+     */
+    CHECK_RETURN_VALUE T * Release()
     {
         T * buffer = ScopedMemoryBuffer<T>::Release();
         mCount     = 0;