Patch Detail
Show a patch.
GET /api/1.1/patches/24091/?format=api
{ "id": 24091, "url": "https://patchwork.libcamera.org/api/1.1/patches/24091/?format=api", "web_url": "https://patchwork.libcamera.org/patch/24091/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20250811131159.1342981-1-barnabas.pocze@ideasonboard.com>", "date": "2025-08-11T13:11:59", "name": "[v1] libcamera: control_serializer: Do not alias byte buffer", "commit_ref": null, "pull_url": null, "state": "new", "archived": false, "hash": "d87734e4dc639aeeac33987444d1143a9242ba87", "submitter": { "id": 216, "url": "https://patchwork.libcamera.org/api/1.1/people/216/?format=api", "name": "Barnabás Pőcze", "email": "barnabas.pocze@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/24091/mbox/", "series": [ { "id": 5366, "url": "https://patchwork.libcamera.org/api/1.1/series/5366/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=5366", "date": "2025-08-11T13:11:59", "name": "[v1] libcamera: control_serializer: Do not alias byte buffer", "version": 1, "mbox": "https://patchwork.libcamera.org/series/5366/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/24091/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/24091/checks/", "tags": {}, "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 96FCABDCC1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 11 Aug 2025 13:12:04 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BDC4C6922A;\n\tMon, 11 Aug 2025 15:12:03 +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 7CED46921E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 11 Aug 2025 15:12:02 +0200 (CEST)", "from pb-laptop.local (185.221.140.182.nat.pool.zt.hu\n\t[185.221.140.182])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5293C4A4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 11 Aug 2025 15:11:10 +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=\"ZY1cvxh6\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1754917870;\n\tbh=FkhbTfoDa+4Ks+g5sXzpN5p6XxHdIo2i9hs7dwQ4oag=;\n\th=From:To:Subject:Date:From;\n\tb=ZY1cvxh6pGhDcLq58cWgsmU5TQ8i3DYChMjGBks5bHAHRnjfecPsbSX9120PQTrDz\n\tnOAcI8a3xB/RziVQEuePBN+NQJTKZ6qXEz64KPFTZm45XTT+KHClHgBk47Ic0ZCZyl\n\tHLueIc/7K/sICG43RD4UtjScN+f+pC6T/hNy2SUo=", "From": "=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Subject": "[PATCH v1] libcamera: control_serializer: Do not alias byte buffer", "Date": "Mon, 11 Aug 2025 15:11:59 +0200", "Message-ID": "<20250811131159.1342981-1-barnabas.pocze@ideasonboard.com>", "X-Mailer": "git-send-email 2.50.1", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=UTF-8", "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>" }, "content": "Aliasing an arbitrary buffer with some `T *` is unlikely to yield good\nresults, primarily due to potential alignment issues. So don't do it,\ninstead create the objects on the stack and read into them, this\nremoves any and all strict aliasing or alignment concerns.\n\nBug: https://bugs.libcamera.org/show_bug.cgi?id=221\nSigned-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n---\n src/libcamera/control_serializer.cpp | 75 +++++++++++++---------------\n 1 file changed, 36 insertions(+), 39 deletions(-)", "diff": "diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp\nindex 050f8512b..722ff231c 100644\n--- a/src/libcamera/control_serializer.cpp\n+++ b/src/libcamera/control_serializer.cpp\n@@ -427,22 +427,22 @@ ControlInfo ControlSerializer::loadControlInfo(ByteStreamBuffer &b)\n template<>\n ControlInfoMap ControlSerializer::deserialize<ControlInfoMap>(ByteStreamBuffer &buffer)\n {\n-\tconst struct ipa_controls_header *hdr = buffer.read<decltype(*hdr)>();\n-\tif (!hdr) {\n+\tstruct ipa_controls_header hdr;\n+\tif (buffer.read(&hdr) < 0) {\n \t\tLOG(Serializer, Error) << \"Out of data\";\n \t\treturn {};\n \t}\n \n-\tauto iter = infoMaps_.find(hdr->handle);\n+\tauto iter = infoMaps_.find(hdr.handle);\n \tif (iter != infoMaps_.end()) {\n \t\tLOG(Serializer, Debug) << \"Use cached ControlInfoMap\";\n \t\treturn iter->second;\n \t}\n \n-\tif (hdr->version != IPA_CONTROLS_FORMAT_VERSION) {\n+\tif (hdr.version != IPA_CONTROLS_FORMAT_VERSION) {\n \t\tLOG(Serializer, Error)\n \t\t\t<< \"Unsupported controls format version \"\n-\t\t\t<< hdr->version;\n+\t\t\t<< hdr.version;\n \t\treturn {};\n \t}\n \n@@ -455,7 +455,7 @@ ControlInfoMap ControlSerializer::deserialize<ControlInfoMap>(ByteStreamBuffer &\n \t */\n \tconst ControlIdMap *idMap = nullptr;\n \tControlIdMap *localIdMap = nullptr;\n-\tswitch (hdr->id_map_type) {\n+\tswitch (hdr.id_map_type) {\n \tcase IPA_CONTROL_ID_MAP_CONTROLS:\n \t\tidMap = &controls::controls;\n \t\tbreak;\n@@ -469,12 +469,12 @@ ControlInfoMap ControlSerializer::deserialize<ControlInfoMap>(ByteStreamBuffer &\n \t\tbreak;\n \tdefault:\n \t\tLOG(Serializer, Error)\n-\t\t\t<< \"Unknown id map type: \" << hdr->id_map_type;\n+\t\t\t<< \"Unknown id map type: \" << hdr.id_map_type;\n \t\treturn {};\n \t}\n \n-\tByteStreamBuffer entries = buffer.carveOut(hdr->data_offset - sizeof(*hdr));\n-\tByteStreamBuffer values = buffer.carveOut(hdr->size - hdr->data_offset);\n+\tByteStreamBuffer entries = buffer.carveOut(hdr.data_offset - sizeof(hdr));\n+\tByteStreamBuffer values = buffer.carveOut(hdr.size - hdr.data_offset);\n \n \tif (buffer.overflow()) {\n \t\tLOG(Serializer, Error) << \"Out of data\";\n@@ -482,36 +482,35 @@ ControlInfoMap ControlSerializer::deserialize<ControlInfoMap>(ByteStreamBuffer &\n \t}\n \n \tControlInfoMap::Map ctrls;\n-\tfor (unsigned int i = 0; i < hdr->entries; ++i) {\n-\t\tconst struct ipa_control_info_entry *entry =\n-\t\t\tentries.read<decltype(*entry)>();\n-\t\tif (!entry) {\n+\tfor (unsigned int i = 0; i < hdr.entries; ++i) {\n+\t\tstruct ipa_control_info_entry entry;\n+\t\tif (entries.read(&entry) < 0) {\n \t\t\tLOG(Serializer, Error) << \"Out of data\";\n \t\t\treturn {};\n \t\t}\n \n-\t\tControlType type = static_cast<ControlType>(entry->type);\n+\t\tControlType type = static_cast<ControlType>(entry.type);\n \n \t\t/* If we're using a local id map, populate it. */\n \t\tif (localIdMap) {\n \t\t\tControlId::DirectionFlags flags{\n-\t\t\t\tstatic_cast<ControlId::Direction>(entry->direction)\n+\t\t\t\tstatic_cast<ControlId::Direction>(entry.direction)\n \t\t\t};\n \n \t\t\t/**\n \t\t\t * \\todo Find a way to preserve the control name for\n \t\t\t * debugging purpose.\n \t\t\t */\n-\t\t\tcontrolIds_.emplace_back(std::make_unique<ControlId>(entry->id,\n+\t\t\tcontrolIds_.emplace_back(std::make_unique<ControlId>(entry.id,\n \t\t\t\t\t\t\t\t\t \"\", \"local\", type,\n \t\t\t\t\t\t\t\t\t flags));\n-\t\t\t(*localIdMap)[entry->id] = controlIds_.back().get();\n+\t\t\t(*localIdMap)[entry.id] = controlIds_.back().get();\n \t\t}\n \n-\t\tconst ControlId *controlId = idMap->at(entry->id);\n+\t\tconst ControlId *controlId = idMap->at(entry.id);\n \t\tASSERT(controlId);\n \n-\t\tif (entry->offset != values.offset()) {\n+\t\tif (entry.offset != values.offset()) {\n \t\t\tLOG(Serializer, Error)\n \t\t\t\t<< \"Bad data, entry offset mismatch (entry \"\n \t\t\t\t<< i << \")\";\n@@ -526,9 +525,9 @@ ControlInfoMap ControlSerializer::deserialize<ControlInfoMap>(ByteStreamBuffer &\n \t * Create the ControlInfoMap in the cache, and store the map to handle\n \t * association.\n \t */\n-\tinfoMaps_[hdr->handle] = ControlInfoMap(std::move(ctrls), *idMap);\n-\tControlInfoMap &map = infoMaps_[hdr->handle];\n-\tinfoMapHandles_[&map] = hdr->handle;\n+\tinfoMaps_[hdr.handle] = ControlInfoMap(std::move(ctrls), *idMap);\n+\tControlInfoMap &map = infoMaps_[hdr.handle];\n+\tinfoMapHandles_[&map] = hdr.handle;\n \n \treturn map;\n }\n@@ -545,21 +544,21 @@ ControlInfoMap ControlSerializer::deserialize<ControlInfoMap>(ByteStreamBuffer &\n template<>\n ControlList ControlSerializer::deserialize<ControlList>(ByteStreamBuffer &buffer)\n {\n-\tconst struct ipa_controls_header *hdr = buffer.read<decltype(*hdr)>();\n-\tif (!hdr) {\n+\tstruct ipa_controls_header hdr;\n+\tif (buffer.read(&hdr) < 0) {\n \t\tLOG(Serializer, Error) << \"Out of data\";\n \t\treturn {};\n \t}\n \n-\tif (hdr->version != IPA_CONTROLS_FORMAT_VERSION) {\n+\tif (hdr.version != IPA_CONTROLS_FORMAT_VERSION) {\n \t\tLOG(Serializer, Error)\n \t\t\t<< \"Unsupported controls format version \"\n-\t\t\t<< hdr->version;\n+\t\t\t<< hdr.version;\n \t\treturn {};\n \t}\n \n-\tByteStreamBuffer entries = buffer.carveOut(hdr->data_offset - sizeof(*hdr));\n-\tByteStreamBuffer values = buffer.carveOut(hdr->size - hdr->data_offset);\n+\tByteStreamBuffer entries = buffer.carveOut(hdr.data_offset - sizeof(hdr));\n+\tByteStreamBuffer values = buffer.carveOut(hdr.size - hdr.data_offset);\n \n \tif (buffer.overflow()) {\n \t\tLOG(Serializer, Error) << \"Out of data\";\n@@ -576,10 +575,10 @@ ControlList ControlSerializer::deserialize<ControlList>(ByteStreamBuffer &buffer\n \t * ControlInfoMap is available.\n \t */\n \tconst ControlIdMap *idMap;\n-\tif (hdr->handle) {\n+\tif (hdr.handle) {\n \t\tauto iter = std::find_if(infoMapHandles_.begin(), infoMapHandles_.end(),\n \t\t\t\t\t [&](decltype(infoMapHandles_)::value_type &entry) {\n-\t\t\t\t\t\t return entry.second == hdr->handle;\n+\t\t\t\t\t\t return entry.second == hdr.handle;\n \t\t\t\t\t });\n \t\tif (iter == infoMapHandles_.end()) {\n \t\t\tLOG(Serializer, Error)\n@@ -590,7 +589,7 @@ ControlList ControlSerializer::deserialize<ControlList>(ByteStreamBuffer &buffer\n \t\tconst ControlInfoMap *infoMap = iter->first;\n \t\tidMap = &infoMap->idmap();\n \t} else {\n-\t\tswitch (hdr->id_map_type) {\n+\t\tswitch (hdr.id_map_type) {\n \t\tcase IPA_CONTROL_ID_MAP_CONTROLS:\n \t\t\tidMap = &controls::controls;\n \t\t\tbreak;\n@@ -601,7 +600,7 @@ ControlList ControlSerializer::deserialize<ControlList>(ByteStreamBuffer &buffer\n \n \t\tcase IPA_CONTROL_ID_MAP_V4L2:\n \t\tdefault:\n-\t\t\tif (hdr->entries > 0)\n+\t\t\tif (hdr.entries > 0)\n \t\t\t\tLOG(Serializer, Fatal)\n \t\t\t\t\t<< \"A list of V4L2 controls requires a ControlInfoMap\";\n \n@@ -617,23 +616,21 @@ ControlList ControlSerializer::deserialize<ControlList>(ByteStreamBuffer &buffer\n \t */\n \tControlList ctrls(*idMap);\n \n-\tfor (unsigned int i = 0; i < hdr->entries; ++i) {\n-\t\tconst struct ipa_control_value_entry *entry =\n-\t\t\tentries.read<decltype(*entry)>();\n-\t\tif (!entry) {\n+\tfor (unsigned int i = 0; i < hdr.entries; ++i) {\n+\t\tstruct ipa_control_value_entry entry;\n+\t\tif (entries.read(&entry) < 0) {\n \t\t\tLOG(Serializer, Error) << \"Out of data\";\n \t\t\treturn {};\n \t\t}\n \n-\t\tif (entry->offset != values.offset()) {\n+\t\tif (entry.offset != values.offset()) {\n \t\t\tLOG(Serializer, Error)\n \t\t\t\t<< \"Bad data, entry offset mismatch (entry \"\n \t\t\t\t<< i << \")\";\n \t\t\treturn {};\n \t\t}\n \n-\t\tctrls.set(entry->id,\n-\t\t\t loadControlValue(values, entry->is_array, entry->count));\n+\t\tctrls.set(entry.id, loadControlValue(values, entry.is_array, entry.count));\n \t}\n \n \treturn ctrls;\n", "prefixes": [ "v1" ] }