From patchwork Mon Aug 11 13:11:59 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 24091 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 96FCABDCC1 for ; Mon, 11 Aug 2025 13:12:04 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id BDC4C6922A; Mon, 11 Aug 2025 15:12:03 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="ZY1cvxh6"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7CED46921E for ; Mon, 11 Aug 2025 15:12:02 +0200 (CEST) Received: from pb-laptop.local (185.221.140.182.nat.pool.zt.hu [185.221.140.182]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 5293C4A4 for ; Mon, 11 Aug 2025 15:11:10 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1754917870; bh=FkhbTfoDa+4Ks+g5sXzpN5p6XxHdIo2i9hs7dwQ4oag=; h=From:To:Subject:Date:From; b=ZY1cvxh6pGhDcLq58cWgsmU5TQ8i3DYChMjGBks5bHAHRnjfecPsbSX9120PQTrDz nOAcI8a3xB/RziVQEuePBN+NQJTKZ6qXEz64KPFTZm45XTT+KHClHgBk47Ic0ZCZyl HLueIc/7K/sICG43RD4UtjScN+f+pC6T/hNy2SUo= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= 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 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" Aliasing an arbitrary buffer with some `T *` is unlikely to yield good results, primarily due to potential alignment issues. So don't do it, instead create the objects on the stack and read into them, this removes any and all strict aliasing or alignment concerns. Bug: https://bugs.libcamera.org/show_bug.cgi?id=221 Signed-off-by: Barnabás Pőcze Reviewed-by: Kieran Bingham --- src/libcamera/control_serializer.cpp | 75 +++++++++++++--------------- 1 file changed, 36 insertions(+), 39 deletions(-) diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp index 050f8512b..722ff231c 100644 --- a/src/libcamera/control_serializer.cpp +++ b/src/libcamera/control_serializer.cpp @@ -427,22 +427,22 @@ ControlInfo ControlSerializer::loadControlInfo(ByteStreamBuffer &b) template<> ControlInfoMap ControlSerializer::deserialize(ByteStreamBuffer &buffer) { - const struct ipa_controls_header *hdr = buffer.read(); - if (!hdr) { + struct ipa_controls_header hdr; + if (buffer.read(&hdr) < 0) { LOG(Serializer, Error) << "Out of data"; return {}; } - auto iter = infoMaps_.find(hdr->handle); + auto iter = infoMaps_.find(hdr.handle); if (iter != infoMaps_.end()) { LOG(Serializer, Debug) << "Use cached ControlInfoMap"; return iter->second; } - if (hdr->version != IPA_CONTROLS_FORMAT_VERSION) { + if (hdr.version != IPA_CONTROLS_FORMAT_VERSION) { LOG(Serializer, Error) << "Unsupported controls format version " - << hdr->version; + << hdr.version; return {}; } @@ -455,7 +455,7 @@ ControlInfoMap ControlSerializer::deserialize(ByteStreamBuffer & */ const ControlIdMap *idMap = nullptr; ControlIdMap *localIdMap = nullptr; - switch (hdr->id_map_type) { + switch (hdr.id_map_type) { case IPA_CONTROL_ID_MAP_CONTROLS: idMap = &controls::controls; break; @@ -469,12 +469,12 @@ ControlInfoMap ControlSerializer::deserialize(ByteStreamBuffer & break; default: LOG(Serializer, Error) - << "Unknown id map type: " << hdr->id_map_type; + << "Unknown id map type: " << hdr.id_map_type; return {}; } - ByteStreamBuffer entries = buffer.carveOut(hdr->data_offset - sizeof(*hdr)); - ByteStreamBuffer values = buffer.carveOut(hdr->size - hdr->data_offset); + ByteStreamBuffer entries = buffer.carveOut(hdr.data_offset - sizeof(hdr)); + ByteStreamBuffer values = buffer.carveOut(hdr.size - hdr.data_offset); if (buffer.overflow()) { LOG(Serializer, Error) << "Out of data"; @@ -482,36 +482,35 @@ ControlInfoMap ControlSerializer::deserialize(ByteStreamBuffer & } ControlInfoMap::Map ctrls; - for (unsigned int i = 0; i < hdr->entries; ++i) { - const struct ipa_control_info_entry *entry = - entries.read(); - if (!entry) { + for (unsigned int i = 0; i < hdr.entries; ++i) { + struct ipa_control_info_entry entry; + if (entries.read(&entry) < 0) { LOG(Serializer, Error) << "Out of data"; return {}; } - ControlType type = static_cast(entry->type); + 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) + static_cast(entry.direction) }; /** * \todo Find a way to preserve the control name for * debugging purpose. */ - controlIds_.emplace_back(std::make_unique(entry->id, + controlIds_.emplace_back(std::make_unique(entry.id, "", "local", type, flags)); - (*localIdMap)[entry->id] = controlIds_.back().get(); + (*localIdMap)[entry.id] = controlIds_.back().get(); } - const ControlId *controlId = idMap->at(entry->id); + const ControlId *controlId = idMap->at(entry.id); ASSERT(controlId); - if (entry->offset != values.offset()) { + if (entry.offset != values.offset()) { LOG(Serializer, Error) << "Bad data, entry offset mismatch (entry " << i << ")"; @@ -526,9 +525,9 @@ ControlInfoMap ControlSerializer::deserialize(ByteStreamBuffer & * Create the ControlInfoMap in the cache, and store the map to handle * association. */ - infoMaps_[hdr->handle] = ControlInfoMap(std::move(ctrls), *idMap); - ControlInfoMap &map = infoMaps_[hdr->handle]; - infoMapHandles_[&map] = hdr->handle; + infoMaps_[hdr.handle] = ControlInfoMap(std::move(ctrls), *idMap); + ControlInfoMap &map = infoMaps_[hdr.handle]; + infoMapHandles_[&map] = hdr.handle; return map; } @@ -545,21 +544,21 @@ ControlInfoMap ControlSerializer::deserialize(ByteStreamBuffer & template<> ControlList ControlSerializer::deserialize(ByteStreamBuffer &buffer) { - const struct ipa_controls_header *hdr = buffer.read(); - if (!hdr) { + struct ipa_controls_header hdr; + if (buffer.read(&hdr) < 0) { LOG(Serializer, Error) << "Out of data"; return {}; } - if (hdr->version != IPA_CONTROLS_FORMAT_VERSION) { + if (hdr.version != IPA_CONTROLS_FORMAT_VERSION) { LOG(Serializer, Error) << "Unsupported controls format version " - << hdr->version; + << hdr.version; return {}; } - ByteStreamBuffer entries = buffer.carveOut(hdr->data_offset - sizeof(*hdr)); - ByteStreamBuffer values = buffer.carveOut(hdr->size - hdr->data_offset); + ByteStreamBuffer entries = buffer.carveOut(hdr.data_offset - sizeof(hdr)); + ByteStreamBuffer values = buffer.carveOut(hdr.size - hdr.data_offset); if (buffer.overflow()) { LOG(Serializer, Error) << "Out of data"; @@ -576,10 +575,10 @@ ControlList ControlSerializer::deserialize(ByteStreamBuffer &buffer * ControlInfoMap is available. */ const ControlIdMap *idMap; - if (hdr->handle) { + if (hdr.handle) { auto iter = std::find_if(infoMapHandles_.begin(), infoMapHandles_.end(), [&](decltype(infoMapHandles_)::value_type &entry) { - return entry.second == hdr->handle; + return entry.second == hdr.handle; }); if (iter == infoMapHandles_.end()) { LOG(Serializer, Error) @@ -590,7 +589,7 @@ ControlList ControlSerializer::deserialize(ByteStreamBuffer &buffer const ControlInfoMap *infoMap = iter->first; idMap = &infoMap->idmap(); } else { - switch (hdr->id_map_type) { + switch (hdr.id_map_type) { case IPA_CONTROL_ID_MAP_CONTROLS: idMap = &controls::controls; break; @@ -601,7 +600,7 @@ ControlList ControlSerializer::deserialize(ByteStreamBuffer &buffer case IPA_CONTROL_ID_MAP_V4L2: default: - if (hdr->entries > 0) + if (hdr.entries > 0) LOG(Serializer, Fatal) << "A list of V4L2 controls requires a ControlInfoMap"; @@ -617,23 +616,21 @@ ControlList ControlSerializer::deserialize(ByteStreamBuffer &buffer */ ControlList ctrls(*idMap); - for (unsigned int i = 0; i < hdr->entries; ++i) { - const struct ipa_control_value_entry *entry = - entries.read(); - if (!entry) { + for (unsigned int i = 0; i < hdr.entries; ++i) { + struct ipa_control_value_entry entry; + if (entries.read(&entry) < 0) { LOG(Serializer, Error) << "Out of data"; return {}; } - if (entry->offset != values.offset()) { + if (entry.offset != values.offset()) { LOG(Serializer, Error) << "Bad data, entry offset mismatch (entry " << i << ")"; return {}; } - ctrls.set(entry->id, - loadControlValue(values, entry->is_array, entry->count)); + ctrls.set(entry.id, loadControlValue(values, entry.is_array, entry.count)); } return ctrls;