[libcamera-devel] [PATCH v2] libcamera: media_device: prevent sign extension on casts

[Kieran Bingham]

The conversion of pointers to integers is implementation defined and
differs between g++ and clang++ when utilising a uint64_t type.

    #include <iostream>

    int main(int argc, char **argv)
    {
        void *ptr = reinterpret_cast<void *>(0xf1234567);
        uint64_t u64 = reinterpret_cast<uint64_t>(ptr);
	uint64_t uint64 = reinterpret_cast<uintptr_t>(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 <kieran.bingham at ideasonboard.com>
---
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 <errno.h>
 #include <fcntl.h>
+#include <stdint.h>
 #include <string>
 #include <string.h>
 #include <sys/ioctl.h>
@@ -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<uintptr_t>(ents);
+		topology.ptr_interfaces = reinterpret_cast<uintptr_t>(interfaces);
+		topology.ptr_links = reinterpret_cast<uintptr_t>(links);
+		topology.ptr_pads = reinterpret_cast<uintptr_t>(pads);
 
 		ret = ioctl(fd_, MEDIA_IOC_G_TOPOLOGY, &topology);
 		if (ret < 0) {
-- 
2.20.1