[{"id":39837,"web_url":"https://patchwork.libcamera.org/comment/39837/","msgid":"<7a931c7b-fb47-454d-88ad-dc42489c4152@ideasonboard.com>","date":"2026-07-24T13:36:06","subject":"Re: [PATCH 2/2] libcamera: Harden control serializer size and input\n\tvalidation","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2026. 07. 23. 19:46 keltezéssel, Magdum írta:\n> Add overflow-safe size computations before writing 32-bit wire fields,\n> centralize control-name size accounting, and validate deserialized\n> local control direction values.\n> \n> Strengthen tests with alignment-safe packet mutation, deterministic\n> bad-terminator corruption, and max-length control-name boundary\n> coverage.\n> \n> Document binarySize() overflow sentinel semantics in the internal\n> serializer header.\n> \n> Signed-off-by: Magdum <magdum.foss@gmail.com>\n> ---\n>   src/libcamera/control_serializer.cpp         | 222 ++++++++++++++++---\n>   test/serialization/control_serialization.cpp |  77 ++++++-\n>   2 files changed, 268 insertions(+), 31 deletions(-)\n> \n> diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp\n> index b3c44ed0..076e78c5 100644\n> --- a/src/libcamera/control_serializer.cpp\n> +++ b/src/libcamera/control_serializer.cpp\n> @@ -8,6 +8,7 @@\n>   #include \"libcamera/internal/control_serializer.h\"\n>   \n>   #include <algorithm>\n> +#include <limits>\n>   #include <memory>\n>   #include <vector>\n>   \n> @@ -50,6 +51,58 @@ bool idMapRequiresLocalIds(enum ipa_controls_id_map_type idMapType)\n>   \treturn idMapType == IPA_CONTROL_ID_MAP_V4L2;\n>   }\n>   \n> +size_t serializedControlNameSize(const ControlId *id,\n> +\t\t\t\t enum ipa_controls_id_map_type idMapType)\n> +{\n> +\treturn idMapRequiresLocalIds(idMapType) ? id->name().size() + 1 : 1;\n> +}\n\nI feel like this function could go into the previous patch.\n\n\n> +\n> +bool fitsU32(size_t value)\n> +{\n> +\treturn value <= std::numeric_limits<uint32_t>::max();\n> +}\n> +\n> +bool safeMulSizeT(size_t lhs, size_t rhs, size_t *out)\n> +{\n> +\tif (!lhs || !rhs) {\n> +\t\t*out = 0;\n> +\t\treturn true;\n> +\t}\n> +\n> +\tif (lhs > std::numeric_limits<size_t>::max() / rhs)\n> +\t\treturn false;\n\nYou can adjust this:\n\n   if (rhs && lhs > std::numeric_limits<size_t>::max() / rhs)\n     return false;\n\nand drop the previous `if`.\n\n\n> +\n> +\t*out = lhs * rhs;\n> +\treturn true;\n> +}\n> +\n> +bool safeAddSizeT(size_t lhs, size_t rhs, size_t *out)\n> +{\n> +\tif (lhs > std::numeric_limits<size_t>::max() - rhs)\n> +\t\treturn false;\n> +\n> +\t*out = lhs + rhs;\n> +\treturn true;\n\nYou can just say `return *out >= lhs;` and drop the `if`.\n\n\n> +}\n\nBut in any case I am a bit wary of these ad-hoc implementations here. Given that\nonly clang and gcc are supported as compilers, I think it would be preferable to\nuse the overflow checking builtins: https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html\nSpecifically `__builtin_{add,mul}_overflow`, possibly hidden inside some trivial\nwrappers in `utils.h`.\n\n\n> +\n> +bool isValidDirection(uint8_t direction)\n> +{\n> +\tconstexpr uint8_t kDirectionIn =\n> +\t\tstatic_cast<uint8_t>(ControlId::Direction::In);\n> +\tconstexpr uint8_t kDirectionOut =\n> +\t\tstatic_cast<uint8_t>(ControlId::Direction::Out);\n\nYou can use `utils::to_underlying()`.\n\n\n> +\tconstexpr uint8_t kDirectionInOut = kDirectionIn | kDirectionOut;\n> +\n> +\tswitch (direction) {\n> +\tcase kDirectionIn:\n> +\tcase kDirectionOut:\n> +\tcase kDirectionInOut:\n\nYou can just say `case (kDirectionIn | kDirectionOut):` here\n\n\n> +\t\treturn true;\n> +\tdefault:\n> +\t\treturn false;\n> +\t}\n\nBut you can also simplify the function greatly, e.g.:\n\n   constexpr unsigned int valid = utils::to_underlying(ControlId::Direction::In) | utils::to_underlying(ControlId::Direction::Out);\n\n   return (direction & valid) && !(direction & ~valid);\n\n\n> +}\n> +\n>   } /* namespace */\n>   \n> [...]","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id C8AE5BDE4C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 24 Jul 2026 13:36:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 80A0C67F1A;\n\tFri, 24 Jul 2026 15:36:08 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 398C767E89\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 24 Jul 2026 15:36:07 +0200 (CEST)","from [192.168.33.42] (185.182.215.156.nat.pool.zt.hu\n\t[185.182.215.156])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 64BF473B;\n\tFri, 24 Jul 2026 15:35:05 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"HFcVCzNm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1784900105;\n\tbh=OY7SojrpWN9qoDD34xctK1fbuCTX5AwDqXO9/PIaIc0=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=HFcVCzNmZ4R5MCp/aZMm8REIM02GUCrK3ibVB8smaxBNVtqkEstwVIrfHZGYtXRbo\n\t7IMWCS1c9Pm+RGaE5fCz7okR2eFDm5HB1SJEzVRuWOauHRVSrDhxtS4uFbZWp3kega\n\t3C93eNI8ormx/wX+rVIUlNHkhqeukQFrSxNIHCiM=","Message-ID":"<7a931c7b-fb47-454d-88ad-dc42489c4152@ideasonboard.com>","Date":"Fri, 24 Jul 2026 15:36:06 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 2/2] libcamera: Harden control serializer size and input\n\tvalidation","To":"Magdum <magdum.foss@gmail.com>, libcamera-devel@lists.libcamera.org","References":"<20260723174644.6580-1-magdum.foss@gmail.com>\n\t<20260723174644.6580-3-magdum.foss@gmail.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<20260723174644.6580-3-magdum.foss@gmail.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":39865,"web_url":"https://patchwork.libcamera.org/comment/39865/","msgid":"<178514391396.8891.16931966941466234670@neptunite.rasen.tech>","date":"2026-07-27T09:18:33","subject":"Re: [PATCH 2/2] libcamera: Harden control serializer size and input\n\tvalidation","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Magdum,\n\nQuoting Magdum (2026-07-24 02:46:44)\n> Add overflow-safe size computations before writing 32-bit wire fields,\n> centralize control-name size accounting, and validate deserialized\n> local control direction values.\n> \n> Strengthen tests with alignment-safe packet mutation, deterministic\n> bad-terminator corruption, and max-length control-name boundary\n> coverage.\n> \n> Document binarySize() overflow sentinel semantics in the internal\n> serializer header.\n> \n> Signed-off-by: Magdum <magdum.foss@gmail.com>\n> ---\n>  src/libcamera/control_serializer.cpp         | 222 ++++++++++++++++---\n>  test/serialization/control_serialization.cpp |  77 ++++++-\n>  2 files changed, 268 insertions(+), 31 deletions(-)\n> \n> diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp\n> index b3c44ed0..076e78c5 100644\n> --- a/src/libcamera/control_serializer.cpp\n> +++ b/src/libcamera/control_serializer.cpp\n> @@ -8,6 +8,7 @@\n>  #include \"libcamera/internal/control_serializer.h\"\n>  \n>  #include <algorithm>\n> +#include <limits>\n>  #include <memory>\n>  #include <vector>\n>  \n> @@ -50,6 +51,58 @@ bool idMapRequiresLocalIds(enum ipa_controls_id_map_type idMapType)\n>         return idMapType == IPA_CONTROL_ID_MAP_V4L2;\n>  }\n>  \n> +size_t serializedControlNameSize(const ControlId *id,\n> +                                enum ipa_controls_id_map_type idMapType)\n> +{\n> +       return idMapRequiresLocalIds(idMapType) ? id->name().size() + 1 : 1;\n> +}\n\nThis looks like it belongs in the previous patch.\n\n> +\n> +bool fitsU32(size_t value)\n> +{\n> +       return value <= std::numeric_limits<uint32_t>::max();\n> +}\n> +\n> +bool safeMulSizeT(size_t lhs, size_t rhs, size_t *out)\n> +{\n> +       if (!lhs || !rhs) {\n> +               *out = 0;\n> +               return true;\n> +       }\n> +\n> +       if (lhs > std::numeric_limits<size_t>::max() / rhs)\n> +               return false;\n> +\n> +       *out = lhs * rhs;\n> +       return true;\n> +}\n> +\n> +bool safeAddSizeT(size_t lhs, size_t rhs, size_t *out)\n> +{\n> +       if (lhs > std::numeric_limits<size_t>::max() - rhs)\n> +               return false;\n> +\n> +       *out = lhs + rhs;\n> +       return true;\n> +}\n> +\n> +bool isValidDirection(uint8_t direction)\n> +{\n> +       constexpr uint8_t kDirectionIn =\n> +               static_cast<uint8_t>(ControlId::Direction::In);\n> +       constexpr uint8_t kDirectionOut =\n> +               static_cast<uint8_t>(ControlId::Direction::Out);\n> +       constexpr uint8_t kDirectionInOut = kDirectionIn | kDirectionOut;\n> +\n> +       switch (direction) {\n> +       case kDirectionIn:\n> +       case kDirectionOut:\n> +       case kDirectionInOut:\n> +               return true;\n> +       default:\n> +               return false;\n> +       }\n> +}\n\nI think Barnabás covered all the points for these helper functions.\n\n> +\n>  } /* namespace */\n>  \n>  /**\n> @@ -184,16 +237,26 @@ size_t ControlSerializer::binarySize(const ControlInfo &info)\n>   */\n>  size_t ControlSerializer::binarySize(const ControlInfoMap &infoMap)\n>  {\n> -       size_t size = sizeof(struct ipa_controls_header)\n> -                   + infoMap.size() * sizeof(struct ipa_control_info_entry);\n\nI think a comment here preserving the above arithmetric will help readability.\n\nSame for the ControlList binarySize().\n\n> +       size_t entriesSize;\n> +       if (!safeMulSizeT(infoMap.size(), sizeof(struct ipa_control_info_entry),\n> +                         &entriesSize))\n> +               return std::numeric_limits<size_t>::max();\n> +\n> +       size_t size;\n> +       if (!safeAddSizeT(sizeof(struct ipa_controls_header), entriesSize, &size))\n> +               return std::numeric_limits<size_t>::max();\n>         enum ipa_controls_id_map_type idMapType = idMapTypeFor(infoMap.idmap());\n>  \n>         for (const auto &ctrl : infoMap) {\n> -               size += binarySize(ctrl.second);\n> -\n> -               size += idMapRequiresLocalIds(idMapType)\n> -                               ? ctrl.first->name().size() + 1\n> -                               : 1;\n> +               size_t nextSize;\n> +               if (!safeAddSizeT(size, binarySize(ctrl.second), &nextSize))\n\nCan't we just override size here instead of using nextSize? If it fails we're\ngoing to bail anyway and if it succeeds then we only care about the new value\nand not about the old value.\n\n> +                       return std::numeric_limits<size_t>::max();\n> +               size = nextSize;\n> +\n> +               size_t nameSize = serializedControlNameSize(ctrl.first, idMapType);\n> +               if (!safeAddSizeT(size, nameSize, &nextSize))\n\nSame here. And every similar occurrence below (there are a lot).\n\n\nPaul\n\n> +                       return std::numeric_limits<size_t>::max();\n> +               size = nextSize;\n>         }\n>  \n>         return size;\n> @@ -210,10 +273,21 @@ size_t ControlSerializer::binarySize(const ControlInfoMap &infoMap)\n>   */\n>  size_t ControlSerializer::binarySize(const ControlList &list)\n>  {\n> -       size_t size = sizeof(struct ipa_controls_header) + list.size() * sizeof(struct ipa_control_list_entry);\n> +       size_t entriesSize;\n> +       if (!safeMulSizeT(list.size(), sizeof(struct ipa_control_list_entry),\n> +                         &entriesSize))\n> +               return std::numeric_limits<size_t>::max();\n>  \n> -       for (const auto &ctrl : list)\n> -               size += binarySize(ctrl.second);\n> +       size_t size;\n> +       if (!safeAddSizeT(sizeof(struct ipa_controls_header), entriesSize, &size))\n> +               return std::numeric_limits<size_t>::max();\n> +\n> +       for (const auto &ctrl : list) {\n> +               size_t nextSize;\n> +               if (!safeAddSizeT(size, binarySize(ctrl.second), &nextSize))\n> +                       return std::numeric_limits<size_t>::max();\n> +               size = nextSize;\n> +       }\n>  \n>         return size;\n>  }\n> @@ -260,23 +334,70 @@ int ControlSerializer::serialize(const ControlInfoMap &infoMap,\n>         enum ipa_controls_id_map_type idMapType = idMapTypeFor(infoMap.idmap());\n>  \n>         /* Compute entries and data required sizes. */\n> -       size_t entriesSize = infoMap.size()\n> -                          * sizeof(struct ipa_control_info_entry);\n> +       size_t entriesSize;\n> +       if (!safeMulSizeT(infoMap.size(), sizeof(struct ipa_control_info_entry),\n> +                         &entriesSize)) {\n> +               LOG(Serializer, Error)\n> +                       << \"ControlInfoMap entries size overflows\";\n> +               return -E2BIG;\n> +       }\n> +\n>         size_t valuesSize = 0;\n>         for (const auto &ctrl : infoMap) {\n> -               valuesSize += binarySize(ctrl.second);\n> -               valuesSize += idMapRequiresLocalIds(idMapType)\n> -                                     ? ctrl.first->name().size() + 1\n> -                                     : 1;\n> +               size_t valueSize = binarySize(ctrl.second);\n> +               size_t nextValuesSize;\n> +               if (!safeAddSizeT(valuesSize, valueSize, &nextValuesSize)) {\n> +                       LOG(Serializer, Error)\n> +                               << \"ControlInfoMap values size overflows\";\n> +                       return -E2BIG;\n> +               }\n> +               valuesSize = nextValuesSize;\n> +\n> +               size_t nameSize = serializedControlNameSize(ctrl.first, idMapType);\n> +               if (!safeAddSizeT(valuesSize, nameSize, &nextValuesSize)) {\n> +                       LOG(Serializer, Error)\n> +                               << \"ControlInfoMap names size overflows\";\n> +                       return -E2BIG;\n> +               }\n> +               valuesSize = nextValuesSize;\n> +       }\n> +\n> +       if (!fitsU32(infoMap.size()) || !fitsU32(entriesSize) ||\n> +           !fitsU32(valuesSize)) {\n> +               LOG(Serializer, Error)\n> +                       << \"ControlInfoMap serialization size exceeds wire limits\";\n> +               return -E2BIG;\n> +       }\n> +\n> +       size_t totalSize;\n> +       if (!safeAddSizeT(sizeof(struct ipa_controls_header), entriesSize,\n> +                         &totalSize) ||\n> +           !safeAddSizeT(totalSize, valuesSize, &totalSize)) {\n> +               LOG(Serializer, Error)\n> +                       << \"ControlInfoMap packet size overflows\";\n> +               return -E2BIG;\n> +       }\n> +\n> +       size_t dataOffset;\n> +       if (!safeAddSizeT(sizeof(struct ipa_controls_header), entriesSize,\n> +                         &dataOffset)) {\n> +               LOG(Serializer, Error)\n> +                       << \"ControlInfoMap data offset overflows\";\n> +               return -E2BIG;\n> +       }\n> +       if (!fitsU32(totalSize) || !fitsU32(dataOffset)) {\n> +               LOG(Serializer, Error)\n> +                       << \"ControlInfoMap packet header exceeds wire limits\";\n> +               return -E2BIG;\n>         }\n>  \n>         /* Prepare the packet header. */\n>         struct ipa_controls_header hdr = {};\n>         hdr.version = IPA_CONTROLS_FORMAT_VERSION;\n>         hdr.handle = serial_;\n> -       hdr.entries = infoMap.size();\n> -       hdr.size = sizeof(hdr) + entriesSize + valuesSize;\n> -       hdr.data_offset = sizeof(hdr) + entriesSize;\n> +       hdr.entries = static_cast<uint32_t>(infoMap.size());\n> +       hdr.size = static_cast<uint32_t>(totalSize);\n> +       hdr.data_offset = static_cast<uint32_t>(dataOffset);\n>         hdr.id_map_type = idMapType;\n>  \n>         buffer.write(&hdr);\n> @@ -384,18 +505,62 @@ int ControlSerializer::serialize(const ControlList &list,\n>         else\n>                 idMapType = IPA_CONTROL_ID_MAP_V4L2;\n>  \n> -       size_t entriesSize = list.size() * sizeof(struct ipa_control_list_entry);\n> +       size_t entriesSize;\n> +       if (!safeMulSizeT(list.size(), sizeof(struct ipa_control_list_entry),\n> +                         &entriesSize)) {\n> +               LOG(Serializer, Error)\n> +                       << \"ControlList entries size overflows\";\n> +               return -E2BIG;\n> +       }\n> +\n>         size_t valuesSize = 0;\n> -       for (const auto &ctrl : list)\n> -               valuesSize += binarySize(ctrl.second);\n> +       for (const auto &ctrl : list) {\n> +               size_t nextValuesSize;\n> +               if (!safeAddSizeT(valuesSize, binarySize(ctrl.second),\n> +                                 &nextValuesSize)) {\n> +                       LOG(Serializer, Error)\n> +                               << \"ControlList values size overflows\";\n> +                       return -E2BIG;\n> +               }\n> +               valuesSize = nextValuesSize;\n> +       }\n> +\n> +       if (!fitsU32(list.size()) || !fitsU32(entriesSize) ||\n> +           !fitsU32(valuesSize)) {\n> +               LOG(Serializer, Error)\n> +                       << \"ControlList serialization size exceeds wire limits\";\n> +               return -E2BIG;\n> +       }\n> +\n> +       size_t totalSize;\n> +       if (!safeAddSizeT(sizeof(struct ipa_controls_header), entriesSize,\n> +                         &totalSize) ||\n> +           !safeAddSizeT(totalSize, valuesSize, &totalSize)) {\n> +               LOG(Serializer, Error)\n> +                       << \"ControlList packet size overflows\";\n> +               return -E2BIG;\n> +       }\n> +\n> +       size_t dataOffset;\n> +       if (!safeAddSizeT(sizeof(struct ipa_controls_header), entriesSize,\n> +                         &dataOffset)) {\n> +               LOG(Serializer, Error)\n> +                       << \"ControlList data offset overflows\";\n> +               return -E2BIG;\n> +       }\n> +       if (!fitsU32(totalSize) || !fitsU32(dataOffset)) {\n> +               LOG(Serializer, Error)\n> +                       << \"ControlList packet header exceeds wire limits\";\n> +               return -E2BIG;\n> +       }\n>  \n>         /* Prepare the packet header. */\n>         struct ipa_controls_header hdr = {};\n>         hdr.version = IPA_CONTROLS_FORMAT_VERSION;\n>         hdr.handle = infoMapHandle;\n> -       hdr.entries = list.size();\n> -       hdr.size = sizeof(hdr) + entriesSize + valuesSize;\n> -       hdr.data_offset = sizeof(hdr) + entriesSize;\n> +       hdr.entries = static_cast<uint32_t>(list.size());\n> +       hdr.size = static_cast<uint32_t>(totalSize);\n> +       hdr.data_offset = static_cast<uint32_t>(dataOffset);\n>         hdr.id_map_type = idMapType;\n>  \n>         buffer.write(&hdr);\n> @@ -578,6 +743,13 @@ ControlInfoMap ControlSerializer::deserialize<ControlInfoMap>(ByteStreamBuffer &\n>  \n>                 /* If we're using a local id map, populate it with the restored name. */\n>                 if (localIdMap) {\n> +                       if (!isValidDirection(entry->direction)) {\n> +                               LOG(Serializer, Error)\n> +                                       << \"Control direction is invalid: \"\n> +                                       << static_cast<unsigned int>(entry->direction);\n> +                               return {};\n> +                       }\n> +\n>                         std::string ctrlName(reinterpret_cast<const char *>(nameData),\n>                                              entry->name_len);\n>  \n> diff --git a/test/serialization/control_serialization.cpp b/test/serialization/control_serialization.cpp\n> index b1638db9..0e15988e 100644\n> --- a/test/serialization/control_serialization.cpp\n> +++ b/test/serialization/control_serialization.cpp\n> @@ -6,6 +6,7 @@\n>   */\n>  \n>  #include <iostream>\n> +#include <string.h>\n>  \n>  #include <libcamera/camera.h>\n>  #include <libcamera/control_ids.h>\n> @@ -224,11 +225,13 @@ protected:\n>  \n>                 /* Reject malformed packets with over-sized names. */\n>                 vector<uint8_t> badNameLenData = infoData;\n> -               auto *badNameLenHeader =\n> -                       reinterpret_cast<ipa_controls_header *>(badNameLenData.data());\n> -               auto *badNameLenEntry = reinterpret_cast<ipa_control_info_entry *>(\n> -                       badNameLenData.data() + sizeof(*badNameLenHeader));\n> -               badNameLenEntry->name_len = 2048;\n> +               ipa_control_info_entry badNameLenEntry;\n> +               std::memcpy(&badNameLenEntry,\n> +                           badNameLenData.data() + sizeof(ipa_controls_header),\n> +                           sizeof(badNameLenEntry));\n> +               badNameLenEntry.name_len = 2048;\n> +               std::memcpy(badNameLenData.data() + sizeof(ipa_controls_header),\n> +                           &badNameLenEntry, sizeof(badNameLenEntry));\n>  \n>                 ControlSerializer badNameLenDeserializer(ControlSerializer::Role::Worker);\n>                 buffer = ByteStreamBuffer(const_cast<const uint8_t *>(badNameLenData.data()),\n> @@ -240,7 +243,30 @@ protected:\n>  \n>                 /* Reject malformed packets with non-null-terminated names. */\n>                 vector<uint8_t> badTermData = infoData;\n> -               badTermData.back() = 'X';\n> +               ipa_controls_header badTermHeader;\n> +               ipa_control_info_entry badTermEntry;\n> +               std::memcpy(&badTermHeader, badTermData.data(), sizeof(badTermHeader));\n> +               std::memcpy(&badTermEntry,\n> +                           badTermData.data() + sizeof(ipa_controls_header),\n> +                           sizeof(badTermEntry));\n> +\n> +               if (badTermEntry.id != kV4L2TestControlId ||\n> +                   badTermEntry.type != ControlTypeInteger32 ||\n> +                   badTermEntry.def.type != ControlTypeInteger32 ||\n> +                   badTermEntry.def.is_array || badTermEntry.def.count != 1 ||\n> +                   badTermEntry.name_len != kV4L2ControlName.size()) {\n> +                       cerr << \"Malformed test packet layout for bad terminator test\" << endl;\n> +                       return TestFail;\n> +               }\n> +\n> +               size_t nameStart = badTermHeader.data_offset + badTermEntry.def.offset +\n> +                                  sizeof(int32_t);\n> +               size_t termOffset = nameStart + badTermEntry.name_len;\n> +               if (termOffset >= badTermData.size()) {\n> +                       cerr << \"Malformed test packet while preparing bad terminator\" << endl;\n> +                       return TestFail;\n> +               }\n> +               badTermData[termOffset] = 'X';\n>  \n>                 ControlSerializer badTermDeserializer(ControlSerializer::Role::Worker);\n>                 buffer = ByteStreamBuffer(const_cast<const uint8_t *>(badTermData.data()),\n> @@ -278,6 +304,45 @@ protected:\n>                         return TestFail;\n>                 }\n>  \n> +               /* Accept names at the configured length limit. */\n> +               vector<unique_ptr<ControlId>> maxNameControlIds;\n> +               ControlIdMap maxNameIdMap;\n> +               string maxName(1024, 'm');\n> +\n> +               maxNameControlIds.emplace_back(std::make_unique<ControlId>(\n> +                       0x009a2003, maxName, \"v4l2\", ControlTypeInteger32,\n> +                       ControlId::Direction::In));\n> +               maxNameIdMap.emplace(0x009a2003, maxNameControlIds.back().get());\n> +\n> +               ControlInfoMap::Map maxNameInfo;\n> +               maxNameInfo.emplace(maxNameControlIds.back().get(),\n> +                                   ControlInfo(ControlValue(int32_t{ 0 }),\n> +                                               ControlValue(int32_t{ 255 }),\n> +                                               ControlValue(int32_t{ 16 })));\n> +               ControlSerializer maxNameSerializer(ControlSerializer::Role::Proxy);\n> +               ControlSerializer maxNameDeserializer(ControlSerializer::Role::Worker);\n> +\n> +               size = maxNameSerializer.binarySize(maxNameInfoMap);\n> +               infoData.resize(size);\n> +               buffer = ByteStreamBuffer(infoData.data(), infoData.size());\n> +\n> +               ret = maxNameSerializer.serialize(maxNameInfoMap, buffer);\n> +               if (ret < 0 || buffer.overflow()) {\n> +                       cerr << \"Max-length control name should serialize successfully\" << endl;\n> +                       return TestFail;\n> +               }\n> +\n> +               buffer = ByteStreamBuffer(const_cast<const uint8_t *>(infoData.data()),\n> +                                         infoData.size());\n> +               ControlInfoMap maxNameInfoMapDes =\n> +                       maxNameDeserializer.deserialize<ControlInfoMap>(buffer);\n> +               auto maxNameIdIt = maxNameInfoMapDes.idmap().find(0x009a2003);\n> +               if (maxNameIdIt == maxNameInfoMapDes.idmap().end() ||\n> +                   maxNameIdIt->second->name() != maxName) {\n> +                       cerr << \"Max-length control name round-trip failed\" << endl;\n> +                       return TestFail;\n> +               }\n> +\n>                 return TestPass;\n>         }\n>  };\n> -- \n> 2.43.0\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 6F544BDE4C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 27 Jul 2026 09:18:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A462667F64;\n\tMon, 27 Jul 2026 11:18:41 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5B4FA67EA6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 27 Jul 2026 11:18:40 +0200 (CEST)","from neptunite.rasen.tech (unknown\n\t[IPv6:2400:2411:160:2f00:b2c4:d1c2:d1f0:43af])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 1A46C3A4;\n\tMon, 27 Jul 2026 11:17:35 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"nYTkvjSF\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1785143856;\n\tbh=cItbAmfbRl4L4x4jjg6UmC+LvfSANL4J4CNhtx8ENEU=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=nYTkvjSFVeYIP1NvDO32Useix1ydvAjXgPyJznqvfugMgpcDIDaT7KkKVPPxWmzf0\n\tHQqa5mfnXaWVYseNn7x0Ow7zEy+bJ6YDmq30rKfBj0FJ6frg+o/Z31sMiPKbAzjBFH\n\txPo44wW88U/n6BhW1xWNabalqrE3JrvjQ9oZBv9g=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20260723174644.6580-3-magdum.foss@gmail.com>","References":"<20260723174644.6580-1-magdum.foss@gmail.com>\n\t<20260723174644.6580-3-magdum.foss@gmail.com>","Subject":"Re: [PATCH 2/2] libcamera: Harden control serializer size and input\n\tvalidation","From":"Paul Elder <paul.elder@ideasonboard.com>","Cc":"Magdum <magdum.foss@gmail.com>","To":"Magdum <magdum.foss@gmail.com>, libcamera-devel@lists.libcamera.org","Date":"Mon, 27 Jul 2026 18:18:33 +0900","Message-ID":"<178514391396.8891.16931966941466234670@neptunite.rasen.tech>","User-Agent":"alot/0.0.0","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]