diff --git a/include/libcamera/ipa/ipa_controls.h b/include/libcamera/ipa/ipa_controls.h index 6af962ff..7a7873d9 100644 --- a/include/libcamera/ipa/ipa_controls.h +++ b/include/libcamera/ipa/ipa_controls.h @@ -15,7 +15,7 @@ namespace libcamera { extern "C" { #endif -#define IPA_CONTROLS_FORMAT_VERSION 2 +#define IPA_CONTROLS_FORMAT_VERSION 3 enum ipa_controls_id_map_type { IPA_CONTROL_ID_MAP_CONTROLS, @@ -50,7 +50,9 @@ struct ipa_control_info_entry { uint32_t id; uint32_t type; uint8_t direction; - uint8_t padding[7]; + uint8_t padding[3]; + /* Length of the control name without the terminating null byte. */ + uint32_t name_len; struct ipa_control_value_entry min; struct ipa_control_value_entry max; struct ipa_control_value_entry def; diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp index c0285cc6..b3c44ed0 100644 --- a/src/libcamera/control_serializer.cpp +++ b/src/libcamera/control_serializer.cpp @@ -31,6 +31,27 @@ namespace libcamera { LOG_DEFINE_CATEGORY(Serializer) +namespace { + +constexpr uint32_t kMaxControlNameLength = 1024; + +enum ipa_controls_id_map_type idMapTypeFor(const ControlIdMap &idmap) +{ + if (&idmap == &controls::controls) + return IPA_CONTROL_ID_MAP_CONTROLS; + if (&idmap == &properties::properties) + return IPA_CONTROL_ID_MAP_PROPERTIES; + + return IPA_CONTROL_ID_MAP_V4L2; +} + +bool idMapRequiresLocalIds(enum ipa_controls_id_map_type idMapType) +{ + return idMapType == IPA_CONTROL_ID_MAP_V4L2; +} + +} /* namespace */ + /** * \class ControlSerializer * \brief Serializer and deserializer for control-related classes @@ -165,10 +186,16 @@ size_t ControlSerializer::binarySize(const ControlInfoMap &infoMap) { size_t size = sizeof(struct ipa_controls_header) + infoMap.size() * sizeof(struct ipa_control_info_entry); + enum ipa_controls_id_map_type idMapType = idMapTypeFor(infoMap.idmap()); - for (const auto &ctrl : infoMap) + for (const auto &ctrl : infoMap) { size += binarySize(ctrl.second); + size += idMapRequiresLocalIds(idMapType) + ? ctrl.first->name().size() + 1 + : 1; + } + return size; } @@ -183,8 +210,7 @@ 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 size = sizeof(struct ipa_controls_header) + list.size() * sizeof(struct ipa_control_list_entry); for (const auto &ctrl : list) size += binarySize(ctrl.second); @@ -231,24 +257,21 @@ int ControlSerializer::serialize(const ControlInfoMap &infoMap, return 0; } + 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 valuesSize = 0; - for (const auto &ctrl : infoMap) + for (const auto &ctrl : infoMap) { valuesSize += binarySize(ctrl.second); - - const ControlIdMap *idmap = &infoMap.idmap(); - enum ipa_controls_id_map_type idMapType; - if (idmap == &controls::controls) - idMapType = IPA_CONTROL_ID_MAP_CONTROLS; - else if (idmap == &properties::properties) - idMapType = IPA_CONTROL_ID_MAP_PROPERTIES; - else - idMapType = IPA_CONTROL_ID_MAP_V4L2; + valuesSize += idMapRequiresLocalIds(idMapType) + ? ctrl.first->name().size() + 1 + : 1; + } /* Prepare the packet header. */ - struct ipa_controls_header hdr; + struct ipa_controls_header hdr = {}; hdr.version = IPA_CONTROLS_FORMAT_VERSION; hdr.handle = serial_; hdr.entries = infoMap.size(); @@ -267,21 +290,29 @@ int ControlSerializer::serialize(const ControlInfoMap &infoMap, */ serial_ += 2; - /* - * Serialize all entries. - * \todo Serialize the control name too - */ + /* Serialize all entries. */ ByteStreamBuffer entries = buffer.carveOut(entriesSize); ByteStreamBuffer values = buffer.carveOut(valuesSize); + static const std::string emptyName; for (const auto &ctrl : infoMap) { const ControlId *id = ctrl.first; const ControlInfo &info = ctrl.second; + const std::string &name = idMapRequiresLocalIds(idMapType) + ? id->name() + : emptyName; + + if (name.size() > kMaxControlNameLength) { + LOG(Serializer, Error) + << "Control name too long: " << name.size(); + return -EINVAL; + } - struct ipa_control_info_entry entry; + struct ipa_control_info_entry entry = {}; entry.id = id->id(); entry.type = id->type(); entry.direction = static_cast(id->direction()); + entry.name_len = static_cast(name.size()); populateControlValueEntry(entry.min, info.min(), values.offset()); store(info.min(), values); @@ -292,6 +323,10 @@ int ControlSerializer::serialize(const ControlInfoMap &infoMap, populateControlValueEntry(entry.def, info.def(), values.offset()); store(info.def(), values); + values.write(Span( + reinterpret_cast(name.c_str()), + name.size() + 1)); + entries.write(&entry); } @@ -355,7 +390,7 @@ int ControlSerializer::serialize(const ControlList &list, valuesSize += binarySize(ctrl.second); /* Prepare the packet header. */ - struct ipa_controls_header hdr; + struct ipa_controls_header hdr = {}; hdr.version = IPA_CONTROLS_FORMAT_VERSION; hdr.handle = infoMapHandle; hdr.entries = list.size(); @@ -485,25 +520,6 @@ ControlInfoMap ControlSerializer::deserialize(ByteStreamBuffer & ControlType type = static_cast(entry->type); - /* If we're using a local id map, populate it. */ - if (localIdMap) { - ControlId::DirectionFlags flags{ - static_cast(entry->direction) - }; - - /** - * \todo Find a way to preserve the control name for - * debugging purpose. - */ - controlIds_.emplace_back(std::make_unique(entry->id, - "", "local", type, - flags)); - (*localIdMap)[entry->id] = controlIds_.back().get(); - } - - const ControlId *controlId = idMap->at(entry->id); - ASSERT(controlId); - const ipa_control_value_entry &min_entry = entry->min; const ipa_control_value_entry &max_entry = entry->max; const ipa_control_value_entry &def_entry = entry->def; @@ -538,6 +554,45 @@ ControlInfoMap ControlSerializer::deserialize(ByteStreamBuffer & loadControlValue(values, static_cast(def_entry.type), def_entry.is_array, def_entry.count); + /* + * Deserialize the null-terminated control name from the values + * section. Reject unreasonably long names to guard against + * malformed packets. + */ + if (entry->name_len > kMaxControlNameLength) { + LOG(Serializer, Error) + << "Control name too long: " << entry->name_len; + return {}; + } + + const auto *nameData = values.read(entry->name_len + 1); + if (!nameData) { + LOG(Serializer, Error) << "Out of data reading control name"; + return {}; + } + + if (nameData[entry->name_len] != '\0') { + LOG(Serializer, Error) << "Control name is not null-terminated"; + return {}; + } + + /* If we're using a local id map, populate it with the restored name. */ + if (localIdMap) { + std::string ctrlName(reinterpret_cast(nameData), + entry->name_len); + + ControlId::DirectionFlags flags{ + static_cast(entry->direction) + }; + + controlIds_.emplace_back(std::make_unique(entry->id, + ctrlName, "local", + type, flags)); + (*localIdMap)[entry->id] = controlIds_.back().get(); + } + + const ControlId *controlId = idMap->at(entry->id); + ASSERT(controlId); /* Create and store the ControlInfo. */ ctrls.emplace(controlId, ControlInfo(min, max, def)); diff --git a/test/serialization/control_serialization.cpp b/test/serialization/control_serialization.cpp index 06c572b7..b1638db9 100644 --- a/test/serialization/control_serialization.cpp +++ b/test/serialization/control_serialization.cpp @@ -11,6 +11,8 @@ #include #include +#include + #include "libcamera/internal/byte_stream_buffer.h" #include "libcamera/internal/control_serializer.h" @@ -169,6 +171,113 @@ protected: return TestFail; } + /* Build a local (V4L2-like) ControlInfoMap and verify name round-trip. */ + vector> v4l2ControlIds; + ControlIdMap v4l2IdMap; + constexpr uint32_t kV4L2TestControlId = 0x009a2001; + const string kV4L2ControlName = "V4L2_CID_TEST_GAIN"; + + v4l2ControlIds.emplace_back(std::make_unique( + kV4L2TestControlId, kV4L2ControlName, "v4l2", + ControlTypeInteger32, ControlId::Direction::In)); + v4l2IdMap.emplace(kV4L2TestControlId, v4l2ControlIds.back().get()); + + ControlInfoMap::Map v4l2Info; + v4l2Info.emplace(v4l2ControlIds.back().get(), + ControlInfo(ControlValue(int32_t{ 0 }), + ControlValue(int32_t{ 255 }), + ControlValue(int32_t{ 16 }))); + ControlInfoMap v4l2InfoMap(std::move(v4l2Info), v4l2IdMap); + + ControlSerializer v4l2Serializer(ControlSerializer::Role::Proxy); + ControlSerializer v4l2Deserializer(ControlSerializer::Role::Worker); + + size = v4l2Serializer.binarySize(v4l2InfoMap); + infoData.resize(size); + buffer = ByteStreamBuffer(infoData.data(), infoData.size()); + + ret = v4l2Serializer.serialize(v4l2InfoMap, buffer); + if (ret < 0 || buffer.overflow()) { + cerr << "Failed to serialize V4L2-like ControlInfoMap" << endl; + return TestFail; + } + + buffer = ByteStreamBuffer(const_cast(infoData.data()), + infoData.size()); + ControlInfoMap v4l2InfoMapDes = + v4l2Deserializer.deserialize(buffer); + if (v4l2InfoMapDes.empty()) { + cerr << "Failed to deserialize V4L2-like ControlInfoMap" << endl; + return TestFail; + } + + auto idIt = v4l2InfoMapDes.idmap().find(kV4L2TestControlId); + if (idIt == v4l2InfoMapDes.idmap().end()) { + cerr << "Deserialized V4L2-like id map misses test control" << endl; + return TestFail; + } + + if (idIt->second->name() != kV4L2ControlName) { + cerr << "Deserialized V4L2-like control name doesn't match" << endl; + return TestFail; + } + + /* Reject malformed packets with over-sized names. */ + vector badNameLenData = infoData; + auto *badNameLenHeader = + reinterpret_cast(badNameLenData.data()); + auto *badNameLenEntry = reinterpret_cast( + badNameLenData.data() + sizeof(*badNameLenHeader)); + badNameLenEntry->name_len = 2048; + + ControlSerializer badNameLenDeserializer(ControlSerializer::Role::Worker); + buffer = ByteStreamBuffer(const_cast(badNameLenData.data()), + badNameLenData.size()); + if (!badNameLenDeserializer.deserialize(buffer).empty()) { + cerr << "Oversized control name should be rejected" << endl; + return TestFail; + } + + /* Reject malformed packets with non-null-terminated names. */ + vector badTermData = infoData; + badTermData.back() = 'X'; + + ControlSerializer badTermDeserializer(ControlSerializer::Role::Worker); + buffer = ByteStreamBuffer(const_cast(badTermData.data()), + badTermData.size()); + if (!badTermDeserializer.deserialize(buffer).empty()) { + cerr << "Control name without null terminator should be rejected" << endl; + return TestFail; + } + + /* Reject too-long names at serialization time. */ + vector> longNameControlIds; + ControlIdMap longNameIdMap; + string longName(1025, 'n'); + + longNameControlIds.emplace_back(std::make_unique( + 0x009a2002, longName, "v4l2", ControlTypeInteger32, + ControlId::Direction::In)); + longNameIdMap.emplace(0x009a2002, longNameControlIds.back().get()); + + ControlInfoMap::Map longNameInfo; + longNameInfo.emplace(longNameControlIds.back().get(), + ControlInfo(ControlValue(int32_t{ 0 }), + ControlValue(int32_t{ 255 }), + ControlValue(int32_t{ 16 }))); + ControlInfoMap longNameInfoMap(std::move(longNameInfo), longNameIdMap); + + ControlSerializer longNameSerializer(ControlSerializer::Role::Proxy); + size = longNameSerializer.binarySize(longNameInfoMap); + infoData.resize(size); + buffer = ByteStreamBuffer(infoData.data(), infoData.size()); + + ret = longNameSerializer.serialize(longNameInfoMap, buffer); + if (ret != -EINVAL) { + cerr << "Too-long control name should fail serialization" << endl; + return TestFail; + } + return TestPass; } };