@@ -508,45 +508,71 @@ 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,
523
+ " PacketBuffer: AvailableSize of a buffer cannot exceed UINT32_MAX. aAvailableSize = 0x" ChipLogFormatX64,
524
+ ChipLogValueX64 (static_cast <uint64_t >(aAvailableSize)));
525
+ return PacketBufferHandle ();
526
+ }
527
+
528
+ // Cast all to uint64_t and add. This cannot overflow because we have
529
+ // ensured that the maximal value of the summation is
530
+ // UINT32_MAX + UINT16_MAX + UINT16_MAX, which should always fit in
531
+ // a uint64_t variable.
532
+ uint64_t sumOfSizes = static_cast <uint64_t >(aAvailableSize) + static_cast <uint64_t >(aReservedSize) +
533
+ static_cast <uint64_t >(PacketBuffer::kStructureSize );
534
+ uint64_t sumOfAvailAndReserved = static_cast <uint64_t >(aAvailableSize) + static_cast <uint64_t >(aReservedSize);
535
+
536
+ // Ensure that the sum fits in a size_t so that casting into size_t variables,
537
+ // viz., lBlockSize and lAllocSize, is safe.
538
+ if (!CanCastTo<size_t >(sumOfSizes))
539
+ {
540
+ ChipLogError (chipSystemLayer,
541
+ " PacketBuffer: Sizes of allocation request are invalid. (aAvailableSize = " ChipLogFormatX64
542
+ " , aReservedSize = " ChipLogFormatX64 " )" ,
543
+ ChipLogValueX64 (static_cast <uint64_t >(aAvailableSize)), ChipLogValueX64 (static_cast <uint64_t >(aReservedSize)));
544
+ return PacketBufferHandle ();
545
+ }
546
+
518
547
#if CHIP_SYSTEM_CONFIG_USE_LWIP
519
548
// LwIP based APIs have a maximum buffer size of UINT16_MAX. Ensure that
520
549
// limit is met during allocation.
521
- VerifyOrDieWithMsg (aAvailableSize + aReservedSize < UINT16_MAX, chipSystemLayer,
522
- " LwIP based systems can handle only up to UINT16_MAX!" );
550
+ VerifyOrDieWithMsg (sumOfAvailAndReserved < UINT16_MAX, chipSystemLayer, " LwIP based systems can handle only up to UINT16_MAX!" );
523
551
#endif // CHIP_SYSTEM_CONFIG_USE_LWIP
524
552
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;
553
+ // sumOfAvailAndReserved is no larger than sumOfSizes, which we checked can be cast to
554
+ // size_t.
555
+ const size_t lAllocSize = static_cast <size_t >(sumOfAvailAndReserved);
529
556
PacketBuffer * lPacket;
530
557
531
558
CHIP_SYSTEM_FAULT_INJECT (FaultInjection::kFault_PacketBufferNew , return PacketBufferHandle ());
532
559
533
- // TODO: Change the max to a lower value
534
- if (aAvailableSize > UINT32_MAX || lAllocSize > PacketBuffer::kMaxSizeWithoutReserve || lBlockSize > UINT32_MAX)
560
+ if (lAllocSize > PacketBuffer::kMaxSizeWithoutReserve )
535
561
{
536
- ChipLogError (chipSystemLayer, " PacketBuffer: allocation too large ." );
562
+ ChipLogError (chipSystemLayer, " PacketBuffer: allocation exceeding buffer capacity limits ." );
537
563
return PacketBufferHandle ();
538
564
}
539
565
540
566
#if CHIP_SYSTEM_CONFIG_USE_LWIP
541
-
567
+ // This cast is safe because lAllocSize is no larger than
568
+ // kMaxSizeWithoutReserve, which fits in uint16_t.
542
569
lPacket = static_cast <PacketBuffer *>(
543
570
pbuf_alloc (PBUF_RAW, static_cast <uint16_t >(lAllocSize), CHIP_SYSTEM_PACKETBUFFER_LWIP_PBUF_TYPE));
544
571
545
572
SYSTEM_STATS_UPDATE_LWIP_PBUF_COUNTS ();
546
573
547
574
#elif CHIP_SYSTEM_PACKETBUFFER_FROM_CHIP_POOL
548
575
549
- static_cast <void >(lBlockSize);
550
576
#if !CHIP_SYSTEM_CONFIG_NO_LOCKING && CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING
551
577
if (!sBufferPoolMutex .isInitialized ())
552
578
{
@@ -565,8 +591,10 @@ PacketBufferHandle PacketBufferHandle::New(size_t aAvailableSize, uint16_t aRese
565
591
UNLOCK_BUF_POOL ();
566
592
567
593
#elif CHIP_SYSTEM_PACKETBUFFER_FROM_CHIP_HEAP
568
-
569
- lPacket = reinterpret_cast <PacketBuffer *>(chip::Platform::MemoryAlloc (lBlockSize));
594
+ // sumOfSizes is essentially (kStructureSize + lAllocSize) which we already
595
+ // checked to fit in a size_t.
596
+ const size_t lBlockSize = static_cast <size_t >(sumOfSizes);
597
+ lPacket = reinterpret_cast <PacketBuffer *>(chip::Platform::MemoryAlloc (lBlockSize));
570
598
SYSTEM_STATS_INCREMENT (chip::System::Stats::kSystemLayer_NumPacketBufs );
571
599
572
600
#else
0 commit comments