Additional PacketBuffer size fixes. (project-chip#8259)

* Additional PacketBuffer size fixes.

#### Problem

1. After project-chip#8226, `PacketBuffer::kMaxSizeWithoutReserve` was wrong on LwIP builds.

2. It is possible for a `PacketBufferHandle::New()` to return a larger
   buffer than requested (e.g. when using a shared pool allocator), and
   in particular for it to return a block that is larger than *can* be
   requested. Attempting `CloneData()` on such a buffer fails with an
   error message logged by `PacketBufferHandle::New()`.

#### Change overview

1. Fix `PacketBuffer::kMaxSizeWithoutReserve`.

2. As long as the excess space is not actually occupied (which would be
   incorrect, since it exceeds the spec size limit), `CloneData()`
   copies correctly to a maximum-size buffer.

#### Testing

Added a unit test to verify that a buffer with excess unused space is
clonable, and a buffer with oversize space in use is not.

* dynamically allocate test buffer

* cast narrowing conversion

src/system/SystemPacketBuffer.cpp

@@ -645,7 +645,22 @@ PacketBufferHandle PacketBufferHandle::CloneData() const
     {
         uint16_t originalDataSize     = original->MaxDataLength();
         uint16_t originalReservedSize = original->ReservedSize();
-        PacketBufferHandle clone      = PacketBufferHandle::New(originalDataSize, originalReservedSize);
+
+        if (originalDataSize + originalReservedSize > PacketBuffer::kMaxSizeWithoutReserve)
+        {
+            // The original memory allocation may have provided a larger block than requested (e.g. when using a shared pool),
+            // and in particular may have provided a larger block than we are able to request from PackBufferHandle::New().
+            // It is a genuine error if that extra space has been used.
+            if (originalReservedSize + original->DataLength() > PacketBuffer::kMaxSizeWithoutReserve)
+            {
+                return PacketBufferHandle();
+            }
+            // Otherwise, reduce the requested data size. This subtraction can not underflow because the above test
+            // guarantees originalReservedSize <= PacketBuffer::kMaxSizeWithoutReserve.
+            originalDataSize = static_cast<uint16_t>(PacketBuffer::kMaxSizeWithoutReserve - originalReservedSize);
+        }
+
+        PacketBufferHandle clone = PacketBufferHandle::New(originalDataSize, originalReservedSize);
         if (clone.IsNull())
         {
             return PacketBufferHandle();

src/system/SystemPacketBuffer.h

@@ -146,7 +146,7 @@ class DLL_EXPORT PacketBuffer : private pbuf
      * The maximum size buffer an application can allocate with no protocol header reserve.
      */
 #if CHIP_SYSTEM_CONFIG_USE_LWIP
-    static constexpr uint16_t kMaxSizeWithoutReserve = (LWIP_MEM_ALIGN_SIZE(PBUF_POOL_BUFSIZE) - PacketBuffer::kStructureSize);
+    static constexpr uint16_t kMaxSizeWithoutReserve = LWIP_MEM_ALIGN_SIZE(PBUF_POOL_BUFSIZE);
 #else
     static constexpr uint16_t kMaxSizeWithoutReserve = CHIP_SYSTEM_CONFIG_PACKETBUFFER_CAPACITY_MAX;
 #endif

src/system/tests/TestSystemPacketBuffer.cpp

@@ -1873,6 +1873,54 @@ void PacketBufferTest::CheckHandleCloneData(nlTestSuite * inSuite, void * inCont
             config_2.handle = nullptr;
         }
     }
+
+#if CHIP_SYSTEM_PACKETBUFFER_STORE == CHIP_SYSTEM_PACKETBUFFER_STORE_CHIP_HEAP
+
+    // It is possible for a packet buffer allocation to return a larger block than requested (e.g. when using a shared pool)
+    // and in particular to return a larger block than it is possible to request from PackBufferHandle::New().
+    // In that case, (a) it is incorrect to actually use the extra space, and (b) if it is not used, the clone will
+    // be the maximum possible size.
+    //
+    // This is only testable on heap allocation configurations, where pbuf records the allocation size and we can manually
+    // construct an oversize buffer.
+
+    constexpr uint16_t kOversizeDataSize = PacketBuffer::kMaxSizeWithoutReserve + 99;
+    PacketBuffer * p =
+        reinterpret_cast<PacketBuffer *>(chip::Platform::MemoryAlloc(PacketBuffer::kStructureSize + kOversizeDataSize));
+    NL_TEST_ASSERT(inSuite, p != nullptr);
+
+    p->next       = nullptr;
+    p->payload    = reinterpret_cast<uint8_t *>(p) + PacketBuffer::kStructureSize;
+    p->tot_len    = 0;
+    p->len        = 0;
+    p->ref        = 1;
+    p->alloc_size = kOversizeDataSize;
+
+    PacketBufferHandle handle = PacketBufferHandle::Adopt(p);
+
+    // Fill the buffer to maximum and verify that it can be cloned.
+
+    memset(handle->Start(), 1, PacketBuffer::kMaxSizeWithoutReserve);
+    handle->SetDataLength(PacketBuffer::kMaxSizeWithoutReserve);
+    NL_TEST_ASSERT(inSuite, handle->DataLength() == PacketBuffer::kMaxSizeWithoutReserve);
+
+    PacketBufferHandle clone = handle.CloneData();
+    NL_TEST_ASSERT(inSuite, !clone.IsNull());
+    NL_TEST_ASSERT(inSuite, clone->DataLength() == PacketBuffer::kMaxSizeWithoutReserve);
+    NL_TEST_ASSERT(inSuite, memcmp(handle->Start(), clone->Start(), PacketBuffer::kMaxSizeWithoutReserve) == 0);
+
+    // Overfill the buffer and verify that it can not be cloned.
+    memset(handle->Start(), 2, kOversizeDataSize);
+    handle->SetDataLength(kOversizeDataSize);
+    NL_TEST_ASSERT(inSuite, handle->DataLength() == kOversizeDataSize);
+
+    clone = handle.CloneData();
+    NL_TEST_ASSERT(inSuite, clone.IsNull());
+
+    // Free the packet buffer memory ourselves, since we allocated it ourselves.
+    chip::Platform::MemoryFree(std::move(handle).UnsafeRelease());
+
+#endif // CHIP_SYSTEM_PACKETBUFFER_STORE == CHIP_SYSTEM_PACKETBUFFER_STORE_CHIP_HEAP
 }
 
 void PacketBufferTest::CheckPacketBufferWriter(nlTestSuite * inSuite, void * inContext)