From 90240a79506a3c400f3af4cb0b08746ae87c79e2 Mon Sep 17 00:00:00 2001 From: Kieran Bingham Date: Tue, 18 Feb 2020 12:51:14 +0000 Subject: libcamera: media_device: prevent sign extension on casts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 << " ptr -> u64 0x" << std::hex << u64 << " ptr -> uintptr_t -> u64 0x" << std::hex << uint64 << std::endl; return 0; } When compiled with g++ for a 32-bit platform produces the following unexpected output: ptr 0xf1234567 ptr -> u64 0xfffffffff1234567 ptr -> uintptr_t -> u64 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 --- 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 e1ae34f8..2c095566 100644 --- a/src/libcamera/media_device.cpp +++ b/src/libcamera/media_device.cpp @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -231,10 +232,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) { -- cgit v1.2.1