@@ -8,6 +8,7 @@
#include "libcamera/internal/control_serializer.h"
#include <algorithm>
+#include <limits>
#include <memory>
#include <vector>
@@ -50,6 +51,58 @@ bool idMapRequiresLocalIds(enum ipa_controls_id_map_type idMapType)
return idMapType == IPA_CONTROL_ID_MAP_V4L2;
}
+size_t serializedControlNameSize(const ControlId *id,
+ enum ipa_controls_id_map_type idMapType)
+{
+ return idMapRequiresLocalIds(idMapType) ? id->name().size() + 1 : 1;
+}
+
+bool fitsU32(size_t value)
+{
+ return value <= std::numeric_limits<uint32_t>::max();
+}
+
+bool safeMulSizeT(size_t lhs, size_t rhs, size_t *out)
+{
+ if (!lhs || !rhs) {
+ *out = 0;
+ return true;
+ }
+
+ if (lhs > std::numeric_limits<size_t>::max() / rhs)
+ return false;
+
+ *out = lhs * rhs;
+ return true;
+}
+
+bool safeAddSizeT(size_t lhs, size_t rhs, size_t *out)
+{
+ if (lhs > std::numeric_limits<size_t>::max() - rhs)
+ return false;
+
+ *out = lhs + rhs;
+ return true;
+}
+
+bool isValidDirection(uint8_t direction)
+{
+ constexpr uint8_t kDirectionIn =
+ static_cast<uint8_t>(ControlId::Direction::In);
+ constexpr uint8_t kDirectionOut =
+ static_cast<uint8_t>(ControlId::Direction::Out);
+ constexpr uint8_t kDirectionInOut = kDirectionIn | kDirectionOut;
+
+ switch (direction) {
+ case kDirectionIn:
+ case kDirectionOut:
+ case kDirectionInOut:
+ return true;
+ default:
+ return false;
+ }
+}
+
} /* namespace */
/**
@@ -184,16 +237,26 @@ size_t ControlSerializer::binarySize(const ControlInfo &info)
*/
size_t ControlSerializer::binarySize(const ControlInfoMap &infoMap)
{
- size_t size = sizeof(struct ipa_controls_header)
- + infoMap.size() * sizeof(struct ipa_control_info_entry);
+ size_t entriesSize;
+ if (!safeMulSizeT(infoMap.size(), sizeof(struct ipa_control_info_entry),
+ &entriesSize))
+ return std::numeric_limits<size_t>::max();
+
+ size_t size;
+ if (!safeAddSizeT(sizeof(struct ipa_controls_header), entriesSize, &size))
+ return std::numeric_limits<size_t>::max();
enum ipa_controls_id_map_type idMapType = idMapTypeFor(infoMap.idmap());
for (const auto &ctrl : infoMap) {
- size += binarySize(ctrl.second);
-
- size += idMapRequiresLocalIds(idMapType)
- ? ctrl.first->name().size() + 1
- : 1;
+ size_t nextSize;
+ if (!safeAddSizeT(size, binarySize(ctrl.second), &nextSize))
+ return std::numeric_limits<size_t>::max();
+ size = nextSize;
+
+ size_t nameSize = serializedControlNameSize(ctrl.first, idMapType);
+ if (!safeAddSizeT(size, nameSize, &nextSize))
+ return std::numeric_limits<size_t>::max();
+ size = nextSize;
}
return size;
@@ -210,10 +273,21 @@ size_t ControlSerializer::binarySize(const ControlInfoMap &infoMap)
*/
size_t ControlSerializer::binarySize(const ControlList &list)
{
- size_t size = sizeof(struct ipa_controls_header) + list.size() * sizeof(struct ipa_control_list_entry);
+ size_t entriesSize;
+ if (!safeMulSizeT(list.size(), sizeof(struct ipa_control_list_entry),
+ &entriesSize))
+ return std::numeric_limits<size_t>::max();
- for (const auto &ctrl : list)
- size += binarySize(ctrl.second);
+ size_t size;
+ if (!safeAddSizeT(sizeof(struct ipa_controls_header), entriesSize, &size))
+ return std::numeric_limits<size_t>::max();
+
+ for (const auto &ctrl : list) {
+ size_t nextSize;
+ if (!safeAddSizeT(size, binarySize(ctrl.second), &nextSize))
+ return std::numeric_limits<size_t>::max();
+ size = nextSize;
+ }
return size;
}
@@ -260,23 +334,70 @@ int ControlSerializer::serialize(const ControlInfoMap &infoMap,
enum ipa_controls_id_map_type idMapType = idMapTypeFor(infoMap.idmap());
/* Compute entries and data required sizes. */
- size_t entriesSize = infoMap.size()
- * sizeof(struct ipa_control_info_entry);
+ size_t entriesSize;
+ if (!safeMulSizeT(infoMap.size(), sizeof(struct ipa_control_info_entry),
+ &entriesSize)) {
+ LOG(Serializer, Error)
+ << "ControlInfoMap entries size overflows";
+ return -E2BIG;
+ }
+
size_t valuesSize = 0;
for (const auto &ctrl : infoMap) {
- valuesSize += binarySize(ctrl.second);
- valuesSize += idMapRequiresLocalIds(idMapType)
- ? ctrl.first->name().size() + 1
- : 1;
+ size_t valueSize = binarySize(ctrl.second);
+ size_t nextValuesSize;
+ if (!safeAddSizeT(valuesSize, valueSize, &nextValuesSize)) {
+ LOG(Serializer, Error)
+ << "ControlInfoMap values size overflows";
+ return -E2BIG;
+ }
+ valuesSize = nextValuesSize;
+
+ size_t nameSize = serializedControlNameSize(ctrl.first, idMapType);
+ if (!safeAddSizeT(valuesSize, nameSize, &nextValuesSize)) {
+ LOG(Serializer, Error)
+ << "ControlInfoMap names size overflows";
+ return -E2BIG;
+ }
+ valuesSize = nextValuesSize;
+ }
+
+ if (!fitsU32(infoMap.size()) || !fitsU32(entriesSize) ||
+ !fitsU32(valuesSize)) {
+ LOG(Serializer, Error)
+ << "ControlInfoMap serialization size exceeds wire limits";
+ return -E2BIG;
+ }
+
+ size_t totalSize;
+ if (!safeAddSizeT(sizeof(struct ipa_controls_header), entriesSize,
+ &totalSize) ||
+ !safeAddSizeT(totalSize, valuesSize, &totalSize)) {
+ LOG(Serializer, Error)
+ << "ControlInfoMap packet size overflows";
+ return -E2BIG;
+ }
+
+ size_t dataOffset;
+ if (!safeAddSizeT(sizeof(struct ipa_controls_header), entriesSize,
+ &dataOffset)) {
+ LOG(Serializer, Error)
+ << "ControlInfoMap data offset overflows";
+ return -E2BIG;
+ }
+ if (!fitsU32(totalSize) || !fitsU32(dataOffset)) {
+ LOG(Serializer, Error)
+ << "ControlInfoMap packet header exceeds wire limits";
+ return -E2BIG;
}
/* Prepare the packet header. */
struct ipa_controls_header hdr = {};
hdr.version = IPA_CONTROLS_FORMAT_VERSION;
hdr.handle = serial_;
- hdr.entries = infoMap.size();
- hdr.size = sizeof(hdr) + entriesSize + valuesSize;
- hdr.data_offset = sizeof(hdr) + entriesSize;
+ hdr.entries = static_cast<uint32_t>(infoMap.size());
+ hdr.size = static_cast<uint32_t>(totalSize);
+ hdr.data_offset = static_cast<uint32_t>(dataOffset);
hdr.id_map_type = idMapType;
buffer.write(&hdr);
@@ -384,18 +505,62 @@ int ControlSerializer::serialize(const ControlList &list,
else
idMapType = IPA_CONTROL_ID_MAP_V4L2;
- size_t entriesSize = list.size() * sizeof(struct ipa_control_list_entry);
+ size_t entriesSize;
+ if (!safeMulSizeT(list.size(), sizeof(struct ipa_control_list_entry),
+ &entriesSize)) {
+ LOG(Serializer, Error)
+ << "ControlList entries size overflows";
+ return -E2BIG;
+ }
+
size_t valuesSize = 0;
- for (const auto &ctrl : list)
- valuesSize += binarySize(ctrl.second);
+ for (const auto &ctrl : list) {
+ size_t nextValuesSize;
+ if (!safeAddSizeT(valuesSize, binarySize(ctrl.second),
+ &nextValuesSize)) {
+ LOG(Serializer, Error)
+ << "ControlList values size overflows";
+ return -E2BIG;
+ }
+ valuesSize = nextValuesSize;
+ }
+
+ if (!fitsU32(list.size()) || !fitsU32(entriesSize) ||
+ !fitsU32(valuesSize)) {
+ LOG(Serializer, Error)
+ << "ControlList serialization size exceeds wire limits";
+ return -E2BIG;
+ }
+
+ size_t totalSize;
+ if (!safeAddSizeT(sizeof(struct ipa_controls_header), entriesSize,
+ &totalSize) ||
+ !safeAddSizeT(totalSize, valuesSize, &totalSize)) {
+ LOG(Serializer, Error)
+ << "ControlList packet size overflows";
+ return -E2BIG;
+ }
+
+ size_t dataOffset;
+ if (!safeAddSizeT(sizeof(struct ipa_controls_header), entriesSize,
+ &dataOffset)) {
+ LOG(Serializer, Error)
+ << "ControlList data offset overflows";
+ return -E2BIG;
+ }
+ if (!fitsU32(totalSize) || !fitsU32(dataOffset)) {
+ LOG(Serializer, Error)
+ << "ControlList packet header exceeds wire limits";
+ return -E2BIG;
+ }
/* Prepare the packet header. */
struct ipa_controls_header hdr = {};
hdr.version = IPA_CONTROLS_FORMAT_VERSION;
hdr.handle = infoMapHandle;
- hdr.entries = list.size();
- hdr.size = sizeof(hdr) + entriesSize + valuesSize;
- hdr.data_offset = sizeof(hdr) + entriesSize;
+ hdr.entries = static_cast<uint32_t>(list.size());
+ hdr.size = static_cast<uint32_t>(totalSize);
+ hdr.data_offset = static_cast<uint32_t>(dataOffset);
hdr.id_map_type = idMapType;
buffer.write(&hdr);
@@ -578,6 +743,13 @@ ControlInfoMap ControlSerializer::deserialize<ControlInfoMap>(ByteStreamBuffer &
/* If we're using a local id map, populate it with the restored name. */
if (localIdMap) {
+ if (!isValidDirection(entry->direction)) {
+ LOG(Serializer, Error)
+ << "Control direction is invalid: "
+ << static_cast<unsigned int>(entry->direction);
+ return {};
+ }
+
std::string ctrlName(reinterpret_cast<const char *>(nameData),
entry->name_len);
@@ -6,6 +6,7 @@
*/
#include <iostream>
+#include <string.h>
#include <libcamera/camera.h>
#include <libcamera/control_ids.h>
@@ -224,11 +225,13 @@ protected:
/* Reject malformed packets with over-sized names. */
vector<uint8_t> badNameLenData = infoData;
- auto *badNameLenHeader =
- reinterpret_cast<ipa_controls_header *>(badNameLenData.data());
- auto *badNameLenEntry = reinterpret_cast<ipa_control_info_entry *>(
- badNameLenData.data() + sizeof(*badNameLenHeader));
- badNameLenEntry->name_len = 2048;
+ ipa_control_info_entry badNameLenEntry;
+ std::memcpy(&badNameLenEntry,
+ badNameLenData.data() + sizeof(ipa_controls_header),
+ sizeof(badNameLenEntry));
+ badNameLenEntry.name_len = 2048;
+ std::memcpy(badNameLenData.data() + sizeof(ipa_controls_header),
+ &badNameLenEntry, sizeof(badNameLenEntry));
ControlSerializer badNameLenDeserializer(ControlSerializer::Role::Worker);
buffer = ByteStreamBuffer(const_cast<const uint8_t *>(badNameLenData.data()),
@@ -240,7 +243,30 @@ protected:
/* Reject malformed packets with non-null-terminated names. */
vector<uint8_t> badTermData = infoData;
- badTermData.back() = 'X';
+ ipa_controls_header badTermHeader;
+ ipa_control_info_entry badTermEntry;
+ std::memcpy(&badTermHeader, badTermData.data(), sizeof(badTermHeader));
+ std::memcpy(&badTermEntry,
+ badTermData.data() + sizeof(ipa_controls_header),
+ sizeof(badTermEntry));
+
+ if (badTermEntry.id != kV4L2TestControlId ||
+ badTermEntry.type != ControlTypeInteger32 ||
+ badTermEntry.def.type != ControlTypeInteger32 ||
+ badTermEntry.def.is_array || badTermEntry.def.count != 1 ||
+ badTermEntry.name_len != kV4L2ControlName.size()) {
+ cerr << "Malformed test packet layout for bad terminator test" << endl;
+ return TestFail;
+ }
+
+ size_t nameStart = badTermHeader.data_offset + badTermEntry.def.offset +
+ sizeof(int32_t);
+ size_t termOffset = nameStart + badTermEntry.name_len;
+ if (termOffset >= badTermData.size()) {
+ cerr << "Malformed test packet while preparing bad terminator" << endl;
+ return TestFail;
+ }
+ badTermData[termOffset] = 'X';
ControlSerializer badTermDeserializer(ControlSerializer::Role::Worker);
buffer = ByteStreamBuffer(const_cast<const uint8_t *>(badTermData.data()),
@@ -278,6 +304,45 @@ protected:
return TestFail;
}
+ /* Accept names at the configured length limit. */
+ vector<unique_ptr<ControlId>> maxNameControlIds;
+ ControlIdMap maxNameIdMap;
+ string maxName(1024, 'm');
+
+ maxNameControlIds.emplace_back(std::make_unique<ControlId>(
+ 0x009a2003, maxName, "v4l2", ControlTypeInteger32,
+ ControlId::Direction::In));
+ maxNameIdMap.emplace(0x009a2003, maxNameControlIds.back().get());
+
+ ControlInfoMap::Map maxNameInfo;
+ maxNameInfo.emplace(maxNameControlIds.back().get(),
+ ControlInfo(ControlValue(int32_t{ 0 }),
+ ControlValue(int32_t{ 255 }),
+ ControlValue(int32_t{ 16 })));
+ ControlSerializer maxNameSerializer(ControlSerializer::Role::Proxy);
+ ControlSerializer maxNameDeserializer(ControlSerializer::Role::Worker);
+
+ size = maxNameSerializer.binarySize(maxNameInfoMap);
+ infoData.resize(size);
+ buffer = ByteStreamBuffer(infoData.data(), infoData.size());
+
+ ret = maxNameSerializer.serialize(maxNameInfoMap, buffer);
+ if (ret < 0 || buffer.overflow()) {
+ cerr << "Max-length control name should serialize successfully" << endl;
+ return TestFail;
+ }
+
+ buffer = ByteStreamBuffer(const_cast<const uint8_t *>(infoData.data()),
+ infoData.size());
+ ControlInfoMap maxNameInfoMapDes =
+ maxNameDeserializer.deserialize<ControlInfoMap>(buffer);
+ auto maxNameIdIt = maxNameInfoMapDes.idmap().find(0x009a2003);
+ if (maxNameIdIt == maxNameInfoMapDes.idmap().end() ||
+ maxNameIdIt->second->name() != maxName) {
+ cerr << "Max-length control name round-trip failed" << endl;
+ return TestFail;
+ }
+
return TestPass;
}
};
Add overflow-safe size computations before writing 32-bit wire fields, centralize control-name size accounting, and validate deserialized local control direction values. Strengthen tests with alignment-safe packet mutation, deterministic bad-terminator corruption, and max-length control-name boundary coverage. Document binarySize() overflow sentinel semantics in the internal serializer header. Signed-off-by: Magdum <magdum.foss@gmail.com> --- src/libcamera/control_serializer.cpp | 222 ++++++++++++++++--- test/serialization/control_serialization.cpp | 77 ++++++- 2 files changed, 268 insertions(+), 31 deletions(-)