[{"id":3806,"web_url":"https://patchwork.libcamera.org/comment/3806/","msgid":"<20200218135459.GE4828@pendragon.ideasonboard.com>","date":"2020-02-18T13:54:59","subject":"Re: [libcamera-devel] [PATCH] libcamera: media_device: prevent sign\n\textension on casts","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nThank you for the patch.\n\nOn Tue, Feb 18, 2020 at 12:51:39PM +0000, Kieran Bingham wrote:\n> Storing the pointer address of the topology structures using a cast to\n> __u64 can fail on 32 bit binaries running on a 64 bit kernel due to sign\n> extension of the pointer.\n\nI really wonder why... I've tested the following code:\n\n void *ptr = reinterpret_cast(0xf1234567);\n uint64_t u64 = reinterpret_cast(ptr);\n\n std::cout << \"ptr \" << ptr << \" u64 0x\" << std::hex << u64 << std::endl;\n\nwith both g++ and clang++. They respectively gave me\n\nptr 0xf1234567 u64 0xfffffffff1234567\nptr 0xf1234567 u64 0xf1234567\n\nI wonder if this is undefined behaviour. The standard says\n\n\"A pointer can be explicitly converted to any integral type large enough\nto hold all values of its type. The mapping function is\nimplementation-defined. [Note: It is intended to be unsurprising to\nthose who know the addressing structure of the underlying machine. — end\nnote]\"\n\nI'd say that gcc is not unsurprising :-)\n\nYou may want to make a note about this in the commit message.\n\n> Convert the casting to use std::uintptr_t which does not sign-extend the value.\n> \n> Signed-off-by: Kieran Bingham \n> ---\n> src/libcamera/media_device.cpp | 8 ++++----\n> 1 file changed, 4 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp\n> index e1ae34f88455..2d493e6c895f 100644\n> --- a/src/libcamera/media_device.cpp\n> +++ b/src/libcamera/media_device.cpp\n> @@ -231,10 +231,10 @@ int MediaDevice::populate()\n> \t */\n> \twhile (true) {\n> \t\ttopology.topology_version = 0;\n> -\t\ttopology.ptr_entities = reinterpret_cast<__u64>(ents);\n> -\t\ttopology.ptr_interfaces = reinterpret_cast<__u64>(interfaces);\n> -\t\ttopology.ptr_links = reinterpret_cast<__u64>(links);\n> -\t\ttopology.ptr_pads = reinterpret_cast<__u64>(pads);\n> +\t\ttopology.ptr_entities = reinterpret_cast(ents);\n> +\t\ttopology.ptr_interfaces = reinterpret_cast(interfaces);\n> +\t\ttopology.ptr_links = reinterpret_cast(links);\n> +\t\ttopology.ptr_pads = reinterpret_cast(pads);\n\nYou need to include stdint for std::uintptr_t. As we use the C-style\nheaders, I recommend stdint.h and uintptr_t.\n\nReviewed-by: Laurent Pinchart \n\n> \n> \t\tret = ioctl(fd_, MEDIA_IOC_G_TOPOLOGY, &topology);\n> \t\tif (ret < 0) {","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 24AF260837\n\tfor ;\n\tTue, 18 Feb 2020 14:55:18 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8D0782F9;\n\tTue, 18 Feb 2020 14:55:17 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1582034117;\n\tbh=G5Cs82GGz+bDZoTuc5fNtD2iY6HDYJQTItDtta6jWvk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=rDAK/kyfOwmgXNj5Kc+HMJwZ5+blFweRPrOac+kFIZvQWKGreGaES+K+DlLjTotv3\n\tjidOgJ410a7jplPWzIzmznFQ/ghlGlFBe+hmUqTRRVqrTk4+OhsP98YewkEJ7xAOGI\n\tpye3odMdRHxIiT8KFobWXUeo2tqhbH4UM9ingyos=","Date":"Tue, 18 Feb 2020 15:54:59 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"libcamera devel ","Message-ID":"<20200218135459.GE4828@pendragon.ideasonboard.com>","References":"<20200218125139.30421-1-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200218125139.30421-1-kieran.bingham@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] libcamera: media_device: prevent sign\n\textension on casts","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 18 Feb 2020 13:55:18 -0000"}}]