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; } };