[libcamera-devel] [PATCH] v4l2: v4l2_camera_proxy: Prevent ioctl sign-extensions
Jacopo Mondi
jacopo.mondi at ideasonboard.com
Tue Jul 11 11:06:17 CEST 2023
Hi Kieran
On Wed, Jul 05, 2023 at 12:22:48AM +0100, Kieran Bingham via libcamera-devel wrote:
> Handling ioctl's within applications is often wrapped with a helper such
> as xioctl. Unfortunately, there are many instances of xioctl which
> incorrectly handle the correct size of the ioctl request.
>
> This leads to incorrect sign-extension of ioctl's which have bit-31 set,
I would expect this if the function arguments were signed, but does
sign-extension happens with unsigned arguments as well ?
> and can cause values to be passed into the libcamera's v4l2 adaptation
> layer which no longer represent the true IOCTL code.
>
> Match the implementation of the Linux kernel and ensure that only 32
> bits of the ioctl request are used by assigning to an unsigned int.
>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> This is a (simpler) rework of the previous patch [0]:
> v4l2: v4l2_camera_proxy: Detect ioctl sign-extensions
>
> [0] https://patchwork.libcamera.org/patch/18311/
>
> src/v4l2/v4l2_camera_proxy.cpp | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index 55ff62cdb430..dfb672080219 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -778,10 +778,20 @@ const std::set<unsigned long> V4L2CameraProxy::supportedIoctls_ = {
> VIDIOC_STREAMOFF,
> };
>
> -int V4L2CameraProxy::ioctl(V4L2CameraFile *file, unsigned long request, void *arg)
> +int V4L2CameraProxy::ioctl(V4L2CameraFile *file, unsigned long longRequest, void *arg)
> {
> MutexLocker locker(proxyMutex_);
>
> + /*
> + * The Linux Kernel only processes 32 bits of an IOCTL.
> + *
> + * Prevent unexpected sign-extensions that could occur if applications
> + * use an unsigned int for the ioctl request, which would sign-extend
> + * to an incorrect value for unsigned longs on 64 bit architectures by
> + * explicitly casting as an unsigned int here.
> + */
> + unsigned int request = longRequest;
> +
> if (!arg && (_IOC_DIR(request) & _IOC_WRITE)) {
> errno = EFAULT;
> return -1;
> --
> 2.34.1
>
More information about the libcamera-devel
mailing list