From patchwork Thu Jul 23 17:46:43 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Magdum X-Patchwork-Id: 27494 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 54021BDE17 for ; Thu, 23 Jul 2026 17:46:55 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 0674F67EDD; Thu, 23 Jul 2026 19:46:55 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="O1O+tzdD"; dkim-atps=neutral Received: from mail-wm1-x32f.google.com (mail-wm1-x32f.google.com [IPv6:2a00:1450:4864:20::32f]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 747C667EC4 for ; Thu, 23 Jul 2026 19:46:53 +0200 (CEST) Received: by mail-wm1-x32f.google.com with SMTP id 5b1f17b1804b1-4954dff6536so7687925e9.0 for ; Thu, 23 Jul 2026 10:46:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1784828813; x=1785433613; darn=lists.libcamera.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to:content-type; bh=j4D5mNG5s332TGUbFpj1QOe0ssQFlCkxx51z8IEuXdQ=; b=O1O+tzdDyprFtq5mWetLFm2JhScWzL6Uve64ZLAjTSwi7ocNkT0guvNeH+XcNe0+8r Q89yf68WT7TDGSCVWBsR7jsAKPWu42vubiocwQmM9DZMZGPBo2siLEyXOs94nGp+7/Oc c/haeIhJEk4uKvniFEugF+A1LTrmtLCXXkxMzelONN/UbeHL+Rvv3n28YYI6pl7d4mq5 9G3Quj2Zx14L0lsvQhxNwAKik5XrHToytXvNgKmx+uboSs4dPiSSSl8hAYtqWrbwOWwL lZz3U8z7b/J96vbXlKKIj23UI/uAusToIDgtudgAfUBzLbFL4Upzbf2JzX0RLZAyP8dg nwQQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1784828813; x=1785433613; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to:content-type; bh=j4D5mNG5s332TGUbFpj1QOe0ssQFlCkxx51z8IEuXdQ=; b=mf36EOB3/jLqz2XgVbvduapfEQmPdXnqJOyaI8kXPg8tsH/LDRITpQv7hLAQ5cwB3q Ug8uKJAZw5qCyzQzkG5BbZjb34at0Rv5felCZUn0vBzKGB7ULXHNP52jIV8S+oXLVsfk vmWxo5j69cVogaXQLb+5WkLg8ztbFASQf3iqFOUPShXlyAVaZCqUFZgscljeyx3uofxJ shca08YnbbaD+TnkwKvAtzBxdEFaWmFXZPQ2C2mRu47mcc53aNKoCuY5DctQQU2zUloS +paLZq/pvv40W//eqhMAXMuvr7UHyCVCyIzfpMLJyks2dHwv4AHqJUXxe09VAOPqu7Nk wSag== X-Gm-Message-State: AOJu0YwdVB9Lw07kXqglfzFSHoDU7wLfz0UKDTV41V/A1wcby0tf4X4F bLsIXnSHJSB536PDQHQKnGj5MV4EW/pY26Ai6aYDRTRBimeJ6Ja+ePD5eQNh/YhAHic= X-Gm-Gg: AR+sD13SQL7qrGwDrEI6cZuDLc4q3UhLUDFvfTOcWNc2xpgO9TGxkifQnY+KjiR6mwN wW7TBUdR5ZWEyYFxF83NrxNMaLmc6EO0b/IwJjeOJczFKGj/AQh6IPKSEm1tNHI0ypC9tQ9KFOe HRm6frgsdrkENUmI4sVzH3sWPNUBw1jNlBR3DhfPlFNCTTPqSjUVehavJNj0yys5WJgnmUInv8U NHVPZk4ILoJ8ZE/56P04sIlUi8awTiR2xduJtUftPq1sk+jsMZZ8Ovj2+2wfiWA6dYKu0GZU1Y0 e5CWYAyw7wyovR547ZiYZM1oPB/adh2T0QyFJT7zqtwlxV0tWCnxUjZu15VNyc1dlRuJ5VNd4tn Ov/q0LDHh+lcmpq1O3tUxEDB7Z9M6jg1bnKOejCE3G9/pinJtsBnYYIjAQT2QRktEIVUtiJ7k6t dFXkYt0hgYFxUr3oeOEGLdZVA37SsL47WwBKdyrg== X-Received: by 2002:a7b:cd14:0:b0:493:bb0:3b43 with SMTP id 5b1f17b1804b1-49573cbfb7fmr32446045e9.2.1784828812765; Thu, 23 Jul 2026 10:46:52 -0700 (PDT) Received: from magdum-System-Product-Name.vodafone.ultrahub ([2a02:810d:4b14:4600:72e8:99a4:5b87:2e43]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-47f85c67339sm18199270f8f.31.2026.07.23.10.46.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 23 Jul 2026 10:46:52 -0700 (PDT) From: Magdum To: libcamera-devel@lists.libcamera.org Cc: Magdum Subject: [PATCH 1/2] libcamera: Serialize local control names in IPA format v3 Date: Thu, 23 Jul 2026 19:46:43 +0200 Message-ID: <20260723174644.6580-2-magdum.foss@gmail.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260723174644.6580-1-magdum.foss@gmail.com> References: <20260723174644.6580-1-magdum.foss@gmail.com> MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Bump IPA controls serialization format to v3 and add name_len to ControlInfoMap entries. Serialize null-terminated control names for local (V4L2-style) id maps to preserve names across IPC. Extend serializer tests with V4L2-like name round-trip coverage and malformed payload rejection for oversized length, missing terminator, and too-long serialize input. Signed-off-by: Magdum --- include/libcamera/ipa/ipa_controls.h | 6 +- src/libcamera/control_serializer.cpp | 133 +++++++++++++------ test/serialization/control_serialization.cpp | 109 +++++++++++++++ 3 files changed, 207 insertions(+), 41 deletions(-) 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; } }; From patchwork Thu Jul 23 17:46:44 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Magdum X-Patchwork-Id: 27495 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id CAD96BDE17 for ; Thu, 23 Jul 2026 17:47:00 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 7CA5167ED8; Thu, 23 Jul 2026 19:47:00 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="PUVhJqoh"; dkim-atps=neutral Received: from mail-wr1-x432.google.com (mail-wr1-x432.google.com [IPv6:2a00:1450:4864:20::432]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E864A67ED2 for ; Thu, 23 Jul 2026 19:46:58 +0200 (CEST) Received: by mail-wr1-x432.google.com with SMTP id ffacd0b85a97d-472326ca506so675538f8f.2 for ; Thu, 23 Jul 2026 10:46:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1784828818; x=1785433618; darn=lists.libcamera.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to:content-type; bh=IDxAOnA7ZYDzq2jbB96zXS1bZl6bPgJG8fFpAKQipDQ=; b=PUVhJqohXp0vp8squbVypByC8O9LCPU6zYjDcjOnwBHPPeJhuK1m5Pv8dBzO7p/Yia cc/fkiHK1/OziZw7Q1Dk0nUImDsjUO4WDOPQL99AxL7ifoRyRI2j7iPBGfOqtU1HKIxt M+uRnwIJFQlg+M2nZcZiOBBdMDffuhD777soumam2OWcJk9lxw/KBiDNgjCWMnhw33df FuZvx3EFdd/zsufm6exX3hrXVywck1vxVDTsQ5fGsbo1Y4T0KoL593ddaBtqq/BRsDxc z1+SfUSfzlk2DSiPr13nGiGVC9slHDSogAki+5+NcjCpAdyJKKISy3DpKQ0DcpL/eZXj fdpQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1784828818; x=1785433618; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to:content-type; bh=IDxAOnA7ZYDzq2jbB96zXS1bZl6bPgJG8fFpAKQipDQ=; b=VLSx6Ci9skceoYoi7JI7r9XW0jEaSGFWhAiosPCP+0EnYuF21Hxe3zCaBb7D1nYBmk iDGiHOt9IGPTXehBzsC3UVyD9xyBCj64ApGoeu0MRQpu9XAsSGeOvCVG10ByM9PN86r0 Lwvmqt/oTnTjpBezW1hVPfGvqK4FLnnzphtOf6IQLD4Wb+ZURsnDzScyP93SfjYKFuzQ 1fsVbJOi1WhkU9hhTglZv+yMsAAn70EsTDQxDvsk5Bs0fqJYS3HK4ls+D/Rh2oKDjjLb l+YNWb5pJUgkWAjgq1qLcmQwyOFay46cgWLrbMF5J6pH7wveaIJ7iOUlZJmDziBxUviP a/sw== X-Gm-Message-State: AOJu0Yy2Zukg5O37+3MyA2SMr7ez28AkY8jE690GTS4A+dmYNkV9qmA+ EM5lCVAkSNQ2sWYmlpEuItQbXpSk0cYpDXkYxQxGbIYfgf21Lz+CXrEfRLHtM7ZqvZk= X-Gm-Gg: AR+sD12UGmfUzG5gUhv1iCUN1QKaJs0+myIrRqYM4sDBbuorMYaDKSUZmPIMdfy2X8Q eSZBtJeXQfUm0hE/ozFZcWxtZ0A4baIKfofDRS+Tq0XQv+YH3vG72K4Rct0b7eNJO5qsEFdfaA1 Z+yoSsHJ1KbbhJNd70DIEmFkZnikk9sdzxgKoYKcl4OJOl5QLevmaSLc8gFpZ6GffYGrajYF5z+ S6aAJrFGdFiA7ZFMm99Du/T5MSVxAC9VUwyiiWKzBR/AzlFy8MaVXLalS4MNmEVFPRkN5fNoRCB TbkTvrSMqlszvfyMjjHcoydGfAbS+P/RBL6pUmeWhdqN6vdfd/O78HbqqAVWnwqxPa49EnAxUwr 0hPKFFYNCETNkM+iAsWVHmsnoBWJEhaZKNVQYqxtMAbwiLfz6p5PLETEwTDJ4LMRU0S79k0iWER rZa+rx5n51Qfdth8rDn3dNG7bIynbxSKkqoEF7YA== X-Received: by 2002:a05:6000:184b:b0:475:f0f0:9eed with SMTP id ffacd0b85a97d-47f8d760185mr5540701f8f.50.1784828818307; Thu, 23 Jul 2026 10:46:58 -0700 (PDT) Received: from magdum-System-Product-Name.vodafone.ultrahub ([2a02:810d:4b14:4600:72e8:99a4:5b87:2e43]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-47f85c67339sm18199270f8f.31.2026.07.23.10.46.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 23 Jul 2026 10:46:57 -0700 (PDT) From: Magdum To: libcamera-devel@lists.libcamera.org Cc: Magdum Subject: [PATCH 2/2] libcamera: Harden control serializer size and input validation Date: Thu, 23 Jul 2026 19:46:44 +0200 Message-ID: <20260723174644.6580-3-magdum.foss@gmail.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260723174644.6580-1-magdum.foss@gmail.com> References: <20260723174644.6580-1-magdum.foss@gmail.com> MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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 --- src/libcamera/control_serializer.cpp | 222 ++++++++++++++++--- test/serialization/control_serialization.cpp | 77 ++++++- 2 files changed, 268 insertions(+), 31 deletions(-) diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp index b3c44ed0..076e78c5 100644 --- a/src/libcamera/control_serializer.cpp +++ b/src/libcamera/control_serializer.cpp @@ -8,6 +8,7 @@ #include "libcamera/internal/control_serializer.h" #include +#include #include #include @@ -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::max(); +} + +bool safeMulSizeT(size_t lhs, size_t rhs, size_t *out) +{ + if (!lhs || !rhs) { + *out = 0; + return true; + } + + if (lhs > std::numeric_limits::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::max() - rhs) + return false; + + *out = lhs + rhs; + return true; +} + +bool isValidDirection(uint8_t direction) +{ + constexpr uint8_t kDirectionIn = + static_cast(ControlId::Direction::In); + constexpr uint8_t kDirectionOut = + static_cast(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::max(); + + size_t size; + if (!safeAddSizeT(sizeof(struct ipa_controls_header), entriesSize, &size)) + return std::numeric_limits::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::max(); + size = nextSize; + + size_t nameSize = serializedControlNameSize(ctrl.first, idMapType); + if (!safeAddSizeT(size, nameSize, &nextSize)) + return std::numeric_limits::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::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::max(); + + for (const auto &ctrl : list) { + size_t nextSize; + if (!safeAddSizeT(size, binarySize(ctrl.second), &nextSize)) + return std::numeric_limits::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(infoMap.size()); + hdr.size = static_cast(totalSize); + hdr.data_offset = static_cast(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(list.size()); + hdr.size = static_cast(totalSize); + hdr.data_offset = static_cast(dataOffset); hdr.id_map_type = idMapType; buffer.write(&hdr); @@ -578,6 +743,13 @@ ControlInfoMap ControlSerializer::deserialize(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(entry->direction); + return {}; + } + std::string ctrlName(reinterpret_cast(nameData), entry->name_len); diff --git a/test/serialization/control_serialization.cpp b/test/serialization/control_serialization.cpp index b1638db9..0e15988e 100644 --- a/test/serialization/control_serialization.cpp +++ b/test/serialization/control_serialization.cpp @@ -6,6 +6,7 @@ */ #include +#include #include #include @@ -224,11 +225,13 @@ protected: /* 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; + 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(badNameLenData.data()), @@ -240,7 +243,30 @@ protected: /* Reject malformed packets with non-null-terminated names. */ vector 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(badTermData.data()), @@ -278,6 +304,45 @@ protected: return TestFail; } + /* Accept names at the configured length limit. */ + vector> maxNameControlIds; + ControlIdMap maxNameIdMap; + string maxName(1024, 'm'); + + maxNameControlIds.emplace_back(std::make_unique( + 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(infoData.data()), + infoData.size()); + ControlInfoMap maxNameInfoMapDes = + maxNameDeserializer.deserialize(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; } };