@@ -508,45 +508,69 @@ void PacketBuffer::AddRef()
508
508
509
509
PacketBufferHandle PacketBufferHandle::New (size_t aAvailableSize, uint16_t aReservedSize)
510
510
{
511
- // Adding three 16-bit-int sized numbers together will never overflow
512
- // assuming int is at least 32 bits.
513
- static_assert (INT_MAX >= INT32_MAX, " int is not big enough" );
511
+ // Sanity check for kStructureSize to ensure that it matches the PacketBuffer size.
514
512
static_assert (PacketBuffer::kStructureSize == sizeof (PacketBuffer), " PacketBuffer size mismatch" );
515
- static_assert (PacketBuffer::kStructureSize < UINT16_MAX, " Check for overflow more carefully" );
516
- static_assert (SIZE_MAX >= INT_MAX, " Our additions might not fit in size_t" );
517
- static_assert (PacketBuffer::kMaxSizeWithoutReserve <= UINT32_MAX, " PacketBuffer may have size not fitting uint32_t" );
513
+ // Setting a static upper bound on kStructureSize to ensure the summation of all the sizes do not overflow.
514
+ static_assert (PacketBuffer::kStructureSize <= UINT16_MAX, " kStructureSize should not exceed UINT16_MAX." );
515
+ // Setting a static upper bound on the maximum buffer size allocation for regular sized messages(not large).
516
+ static_assert (PacketBuffer::kMaxSizeWithoutReserve <= UINT16_MAX, " kMaxSizeWithoutReserve should not exceed UINT16_MAX." );
517
+
518
+ // Ensure that aAvailableSize is bound within a max and is not big enough to cause overflow during
519
+ // subsequent addtion of all the sizes.
520
+ if (aAvailableSize > UINT32_MAX)
521
+ {
522
+ ChipLogError (chipSystemLayer, " PacketBuffer: AvailableSize of a buffer cannot exceed UINT32_MAX. aAvailableSize = %u" ,
523
+ static_cast <unsigned >(aAvailableSize));
524
+ return PacketBufferHandle ();
525
+ }
526
+
527
+ // Cast all to uint64_t and add. This cannot overflow because we have
528
+ // ensured that the maximal value of the summation is
529
+ // UINT32_MAX + UINT16_MAX + UINT16_MAX, which should always fit in
530
+ // a uint64_t variable.
531
+ uint64_t sumOfSizes = static_cast <uint64_t >(aAvailableSize) + static_cast <uint64_t >(aReservedSize) +
532
+ static_cast <uint64_t >(PacketBuffer::kStructureSize );
533
+ uint64_t sumOfAvailAndReserved = static_cast <uint64_t >(aAvailableSize) + static_cast <uint64_t >(aReservedSize);
534
+
535
+ // Ensure that the sum fits in a size_t so that casting into size_t variables,
536
+ // viz., lBlockSize and lAllocSize, is safe.
537
+ if (!CanCastTo<size_t >(sumOfSizes))
538
+ {
539
+ ChipLogError (chipSystemLayer,
540
+ " PacketBuffer: Sizes of allocation request are invalid. (aAvailableSize = %u, aReservedSize = %u)" ,
541
+ static_cast <unsigned >(aAvailableSize), static_cast <unsigned >(aReservedSize));
542
+ return PacketBufferHandle ();
543
+ }
544
+
518
545
#if CHIP_SYSTEM_CONFIG_USE_LWIP
519
546
// LwIP based APIs have a maximum buffer size of UINT16_MAX. Ensure that
520
547
// limit is met during allocation.
521
- VerifyOrDieWithMsg (aAvailableSize + aReservedSize < UINT16_MAX, chipSystemLayer,
522
- " LwIP based systems can handle only up to UINT16_MAX!" );
548
+ VerifyOrDieWithMsg (sumOfAvailAndReserved < UINT16_MAX, chipSystemLayer, " LwIP based systems can handle only up to UINT16_MAX!" );
523
549
#endif // CHIP_SYSTEM_CONFIG_USE_LWIP
524
550
525
- // When `aAvailableSize` fits in uint16_t (as tested below) and size_t is at least 32 bits (as asserted above),
526
- // these additions will not overflow.
527
- const size_t lAllocSize = aReservedSize + aAvailableSize;
528
- const size_t lBlockSize = PacketBuffer::kStructureSize + lAllocSize;
551
+ // sumOfAvailAndReserved is no larger than sumOfSizes, which we checked can be cast to
552
+ // size_t.
553
+ const size_t lAllocSize = static_cast <size_t >(sumOfAvailAndReserved);
529
554
PacketBuffer * lPacket;
530
555
531
556
CHIP_SYSTEM_FAULT_INJECT (FaultInjection::kFault_PacketBufferNew , return PacketBufferHandle ());
532
557
533
- // TODO: Change the max to a lower value
534
- if (aAvailableSize > UINT32_MAX || lAllocSize > PacketBuffer::kMaxSizeWithoutReserve || lBlockSize > UINT32_MAX)
558
+ if (lAllocSize > PacketBuffer::kMaxSizeWithoutReserve )
535
559
{
536
- ChipLogError (chipSystemLayer, " PacketBuffer: allocation too large ." );
560
+ ChipLogError (chipSystemLayer, " PacketBuffer: allocation exceeding buffer capacity limits ." );
537
561
return PacketBufferHandle ();
538
562
}
539
563
540
564
#if CHIP_SYSTEM_CONFIG_USE_LWIP
541
-
565
+ // This cast is safe because lAllocSize is no larger than
566
+ // kMaxSizeWithoutReserve, which fits in uint16_t.
542
567
lPacket = static_cast <PacketBuffer *>(
543
568
pbuf_alloc (PBUF_RAW, static_cast <uint16_t >(lAllocSize), CHIP_SYSTEM_PACKETBUFFER_LWIP_PBUF_TYPE));
544
569
545
570
SYSTEM_STATS_UPDATE_LWIP_PBUF_COUNTS ();
546
571
547
572
#elif CHIP_SYSTEM_PACKETBUFFER_FROM_CHIP_POOL
548
573
549
- static_cast <void >(lBlockSize);
550
574
#if !CHIP_SYSTEM_CONFIG_NO_LOCKING && CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING
551
575
if (!sBufferPoolMutex .isInitialized ())
552
576
{
@@ -565,8 +589,10 @@ PacketBufferHandle PacketBufferHandle::New(size_t aAvailableSize, uint16_t aRese
565
589
UNLOCK_BUF_POOL ();
566
590
567
591
#elif CHIP_SYSTEM_PACKETBUFFER_FROM_CHIP_HEAP
568
-
569
- lPacket = reinterpret_cast <PacketBuffer *>(chip::Platform::MemoryAlloc (lBlockSize));
592
+ // sumOfSizes is essentially (kStructureSize + lAllocSize) which we already
593
+ // checked to fit in a size_t.
594
+ const size_t lBlockSize = static_cast <size_t >(sumOfSizes);
595
+ lPacket = reinterpret_cast <PacketBuffer *>(chip::Platform::MemoryAlloc (lBlockSize));
570
596
SYSTEM_STATS_INCREMENT (chip::System::Stats::kSystemLayer_NumPacketBufs );
571
597
572
598
#else
0 commit comments