From patchwork Tue Feb 18 14:18:39 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Kieran Bingham X-Patchwork-Id: 2858 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E34FD60837 for ; Tue, 18 Feb 2020 15:18:42 +0100 (CET) Received: from localhost.localdomain (cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 4F55F2F9; Tue, 18 Feb 2020 15:18:42 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1582035522; bh=bvGf19VUVpTEvuBqmphl5eQG+Xaks9QZTL6fF2OvU6k=; h=From:To:Cc:Subject:Date:From; b=ZJwjENIWA/ydUZUxfNMp+dPiCvRZGLrjYM1sQCWZ7oq4fylsRyXGdMR4ijwQbXAHD awP3xJgmjhbRvjt318JrDDMhJBy/MLUefykVPwYdFEgnRzIrZOgS2g+3RWDF2GvSbk zvu6b3+ygLkJGB29VKvpgBx981sRVBxwy+4/V6bQ= From: Kieran Bingham To: libcamera devel Date: Tue, 18 Feb 2020 14:18:39 +0000 Message-Id: <20200218141839.5574-1-kieran.bingham@ideasonboard.com> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2] libcamera: media_device: prevent sign extension on casts 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: , X-List-Received-Date: Tue, 18 Feb 2020 14:18:43 -0000 The conversion of pointers to integers is implementation defined and differs between g++ and clang++ when utilising a uint64_t type. #include int main(int argc, char **argv) { void *ptr = reinterpret_cast(0xf1234567); uint64_t u64 = reinterpret_cast(ptr); uint64_t uint64 = reinterpret_cast(ptr); std::cout << "ptr " << ptr << " u64 0x" << std::hex << u64 << " uint64 0x" << std::hex << uint64 << std::endl; return 0; } When compiled with g++ produces the following unexpected output: ptr 0xf1234567 u64 0xfffffffff1234567 uint64 0xf1234567 The standards states: "A pointer can be explicitly converted to any integral type large enough to hold all values of its type. The mapping function is implementation-defined. [Note: It is intended to be unsurprising to those who know the addressing structure of the underlying machine. — end note]" And as such the g++ implementation appears to be little more surprising than expected in this situation. The MediaDevice passes pointers to the kernel via the struct media_v2_topology in which pointers are cast using a uint64 type (__u64), which is affected by the sign extension described above when BIT(32) is set and causes an invalid address to be given to the kernel. Ensure that we cast using uintptr_t which is not affected by the sign extension issue. Signed-off-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- v2: - Expand commit message to explain the underlying issue. - include stdint.h - use uintptr_t instead of std::uintptr_t src/libcamera/media_device.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp index fad475b9ac76..0d6b5efd9e7a 100644 --- a/src/libcamera/media_device.cpp +++ b/src/libcamera/media_device.cpp @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -236,10 +237,10 @@ int MediaDevice::populate() */ while (true) { topology.topology_version = 0; - topology.ptr_entities = reinterpret_cast<__u64>(ents); - topology.ptr_interfaces = reinterpret_cast<__u64>(interfaces); - topology.ptr_links = reinterpret_cast<__u64>(links); - topology.ptr_pads = reinterpret_cast<__u64>(pads); + topology.ptr_entities = reinterpret_cast(ents); + topology.ptr_interfaces = reinterpret_cast(interfaces); + topology.ptr_links = reinterpret_cast(links); + topology.ptr_pads = reinterpret_cast(pads); ret = ioctl(fd_, MEDIA_IOC_G_TOPOLOGY, &topology); if (ret < 0) {