Added validateMemoryLayout() function to Frame::Plane

Summary: This new function allows to validate whether a plane's memory is out of bounds and whether the corresponding Frame needs to be treated as invalid. Reviewed By: enpe Differential Revision: D68121395 fbshipit-source-id: dd65616b32b86fe63e2169ed6d11479379049a2a
facebookresearch · Jan 14, 2025 · 69d8d23 · 69d8d23
1 parent 3b6d345
commit 69d8d23
Show file tree

Hide file tree

Showing 2 changed files with 80 additions and 17 deletions.
diff --git a/impl/ocean/base/Frame.cpp b/impl/ocean/base/Frame.cpp
@@ -2336,37 +2336,58 @@ Frame::Frame(const FrameType& frameType, const PlaneInitializer<void>* planeInit
 			{
 				const PlaneInitializer<void>& planeInitializer = planeInitializers[planeIndex];
 
-				if (planeInitializer.data_ == nullptr && planeInitializer.constdata_ == nullptr)
+				if (Plane::validateMemoryLayout(planeWidth, planeHeight, planeChannels, bytesPerElement, planeInitializer.paddingElements_))
 				{
-					planes_.pushBack(Plane(planeWidth, planeHeight, planeChannels, bytesPerElement, planeInitializer.paddingElements_));
-				}
-				else
-				{
-					if (planeInitializer.copyMode_ == CM_USE_KEEP_LAYOUT)
+					if (planeInitializer.data_ == nullptr && planeInitializer.constdata_ == nullptr)
+					{
+						planes_.pushBack(Plane(planeWidth, planeHeight, planeChannels, bytesPerElement, planeInitializer.paddingElements_));
+					}
+					else
 					{
-						if (planeInitializer.data_ != nullptr)
+						if (planeInitializer.copyMode_ == CM_USE_KEEP_LAYOUT)
 						{
-							planes_.pushBack(Plane(planeWidth, planeHeight, planeChannels, bytesPerElement, planeInitializer.data_, planeInitializer.paddingElements_));
+							if (planeInitializer.data_ != nullptr)
+							{
+								planes_.pushBack(Plane(planeWidth, planeHeight, planeChannels, bytesPerElement, planeInitializer.data_, planeInitializer.paddingElements_));
+							}
+							else
+							{
+								ocean_assert(planeInitializer.constdata_ != nullptr);
+								planes_.pushBack(Plane(planeWidth, planeHeight, planeChannels, bytesPerElement, planeInitializer.constdata_, planeInitializer.paddingElements_));
+							}
 						}
 						else
 						{
-							ocean_assert(planeInitializer.constdata_ != nullptr);
-							planes_.pushBack(Plane(planeWidth, planeHeight, planeChannels, bytesPerElement, planeInitializer.constdata_, planeInitializer.paddingElements_));
+							const void* const data = planeInitializer.constdata_ ? planeInitializer.constdata_ : planeInitializer.data_;
+							ocean_assert(data != nullptr);
+
+							planes_.pushBack(Plane(planeWidth, planeHeight, planeChannels, bytesPerElement, data, planeInitializer.paddingElements_, planeInitializer.copyMode_));
 						}
 					}
-					else
-					{
-						const void* const data = planeInitializer.constdata_ ? planeInitializer.constdata_ : planeInitializer.data_;
-						ocean_assert(data != nullptr);
+				}
+				else
+				{
+					ocean_assert(false && "Invalid frame type!");
 
-						planes_.pushBack(Plane(planeWidth, planeHeight, planeChannels, bytesPerElement, data, planeInitializer.paddingElements_, planeInitializer.copyMode_));
-					}
+					release();
+					return;
 				}
 			}
 			else
 			{
 				constexpr unsigned int paddingElements = 0u;
-				planes_.pushBack(Plane(planeWidth, planeHeight, planeChannels, bytesPerElement, paddingElements));
+
+				if (Plane::validateMemoryLayout(planeWidth, planeHeight, planeChannels, bytesPerElement, paddingElements))
+				{
+					planes_.pushBack(Plane(planeWidth, planeHeight, planeChannels, bytesPerElement, paddingElements));
+				}
+				else
+				{
+					ocean_assert(false && "Invalid frame type!");
+
+					release();
+					return;
+				}
 			}
 		}
 		else

diff --git a/impl/ocean/base/Frame.h b/impl/ocean/base/Frame.h
@@ -2133,6 +2133,17 @@ class OCEAN_BASE_EXPORT Frame : public FrameType
 				 */
 				static void* alignedMemory(const size_t size, const size_t alignment, void*& alignedData);
 
+				/**
+				 * Returns whether the memory layout of a plane is valid (and fits into the memory).
+				 * @param planeWidth The width of the plane, in pixel, with range [0, infinity)
+				 * @param planeHeight The height of the plane, in pixel, with range [0, infinity)
+				 * @param planeChannels The channels of the plane, with range [0, infinity)
+				 * @param bytesPerElement The number of bytes each element has, with range [1, infinity)
+				 * @param paddingElements The optional number of padding elements at the end of each plane row, in elements, with range [0, infinity)
+				 * @return True, if so; False, if the memory usage is out of bounds
+				 */
+				static constexpr bool validateMemoryLayout(const unsigned int planeWidth, const unsigned int planeHeight, const unsigned int planeChannels, const unsigned int bytesPerElement, const unsigned int paddingElements);
+
 			protected:
 
 				/**
@@ -3664,6 +3675,37 @@ inline bool Frame::Plane::isValid() const
 	return width_ != 0u && height_ != 0u && channels_ != 0u;
 }
 
+constexpr bool Frame::Plane::validateMemoryLayout(const unsigned int planeWidth, const unsigned int planeHeight, const unsigned int planeChannels, const unsigned int bytesPerElement, const unsigned int paddingElements)
+{
+	if (!isProductInsideValueRange(planeWidth, planeChannels))
+	{
+		return false;
+	}
+
+	const unsigned int planeWidthElements = planeWidth * planeChannels;
+
+	if (!isSumInsideValueRange(planeWidthElements, paddingElements))
+	{
+		return false;
+	}
+
+	const unsigned int planeStideElements = planeWidthElements + paddingElements;
+
+	if (!isProductInsideValueRange(planeStideElements, bytesPerElement))
+	{
+		return false;
+	}
+
+	const unsigned int planeStrideBytes = planeStideElements * bytesPerElement;
+
+	if (!isProductInsideValueRange(planeStrideBytes, planeHeight))
+	{
+		return false;
+	}
+
+	return true;
+}
+
 inline unsigned int Frame::Plane::calculateStrideBytes() const
 {
 	ocean_assert(isProductInsideValueRange(strideElements(), elementTypeSize_));