linux-kernel - Re: [PATCH] media: v4l2-core: fix VIDIOC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Mon, 8 Nov 2021 12:19:23 +0100
From:   Hans Verkuil <hverkuil-cisco@...all.nl>
To:     Arnd Bergmann <arnd@...nel.org>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Arnd Bergmann <arnd@...db.de>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc:     John Stultz <john.stultz@...aro.org>,
        Sergey Senozhatsky <senozhatsky@...omium.org>,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] media: v4l2-core: fix VIDIOC_DQEVENT handling on non-x86

Hi Arnd,

I tested this and it looks good.

One question: is support for arch/x86 (x86_64-linux-muslx32-native) supposed to work?
It fails for me when I test this. It's not related to this patch, it is failing for
pretty much any compat ioctl.

In any case, I'm accepting this patch.

Regards,

	Hans

On 26/10/2021 07:49, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@...db.de>
> 
> My previous bugfix addressed an API inconsistency found by syzbot,
> and it correctly fixed the issue on x86-64 machines, which now behave
> correctly for both native and compat tasks.
> 
> Unfortunately, John found that the patch broke compat mode on all other
> architectures, as they can no longer rely on the VIDIOC_DQEVENT_TIME32
> code from the native handler as a fallback in the compat code.
> 
> The best way I can see for addressing this is to generalize the
> VIDIOC_DQEVENT32_TIME32 code from x86 and use that for all architectures,
> leaving only the VIDIOC_DQEVENT32 variant as x86 specific. The original
> code was trying to be clever and use the same conversion helper for native
> 32-bit code and compat mode, but that turned out to be too obscure so
> even I missed that bit I had introduced myself when I made the fix.
> 
> Fixes: c344f07aa1b4 ("media: v4l2-core: ignore native time32 ioctls on 64-bit")
> Reported-by: John Stultz <john.stultz@...aro.org>
> Signed-off-by: Arnd Bergmann <arnd@...db.de>
> ---
>  drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 41 ++++++++-----------
>  1 file changed, 17 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> index 8176769a89fa..0f3d6b5667b0 100644
> --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> @@ -751,10 +751,6 @@ static int put_v4l2_ext_controls32(struct v4l2_ext_controls *p64,
>  /*
>   * x86 is the only compat architecture with different struct alignment
>   * between 32-bit and 64-bit tasks.
> - *
> - * On all other architectures, v4l2_event32 and v4l2_event32_time32 are
> - * the same as v4l2_event and v4l2_event_time32, so we can use the native
> - * handlers, converting v4l2_event to v4l2_event_time32 if necessary.
>   */
>  struct v4l2_event32 {
>  	__u32				type;
> @@ -772,21 +768,6 @@ struct v4l2_event32 {
>  	__u32				reserved[8];
>  };
>  
> -#ifdef CONFIG_COMPAT_32BIT_TIME
> -struct v4l2_event32_time32 {
> -	__u32				type;
> -	union {
> -		compat_s64		value64;
> -		__u8			data[64];
> -	} u;
> -	__u32				pending;
> -	__u32				sequence;
> -	struct old_timespec32		timestamp;
> -	__u32				id;
> -	__u32				reserved[8];
> -};
> -#endif
> -
>  static int put_v4l2_event32(struct v4l2_event *p64,
>  			    struct v4l2_event32 __user *p32)
>  {
> @@ -802,7 +783,22 @@ static int put_v4l2_event32(struct v4l2_event *p64,
>  	return 0;
>  }
>  
> +#endif
> +
>  #ifdef CONFIG_COMPAT_32BIT_TIME
> +struct v4l2_event32_time32 {
> +	__u32				type;
> +	union {
> +		compat_s64		value64;
> +		__u8			data[64];
> +	} u;
> +	__u32				pending;
> +	__u32				sequence;
> +	struct old_timespec32		timestamp;
> +	__u32				id;
> +	__u32				reserved[8];
> +};
> +
>  static int put_v4l2_event32_time32(struct v4l2_event *p64,
>  				   struct v4l2_event32_time32 __user *p32)
>  {
> @@ -818,7 +814,6 @@ static int put_v4l2_event32_time32(struct v4l2_event *p64,
>  	return 0;
>  }
>  #endif
> -#endif
>  
>  struct v4l2_edid32 {
>  	__u32 pad;
> @@ -880,9 +875,7 @@ static int put_v4l2_edid32(struct v4l2_edid *p64,
>  #define VIDIOC_QUERYBUF32_TIME32	_IOWR('V',  9, struct v4l2_buffer32_time32)
>  #define VIDIOC_QBUF32_TIME32		_IOWR('V', 15, struct v4l2_buffer32_time32)
>  #define VIDIOC_DQBUF32_TIME32		_IOWR('V', 17, struct v4l2_buffer32_time32)
> -#ifdef CONFIG_X86_64
>  #define	VIDIOC_DQEVENT32_TIME32		_IOR ('V', 89, struct v4l2_event32_time32)
> -#endif
>  #define VIDIOC_PREPARE_BUF32_TIME32	_IOWR('V', 93, struct v4l2_buffer32_time32)
>  #endif
>  
> @@ -936,10 +929,10 @@ unsigned int v4l2_compat_translate_cmd(unsigned int cmd)
>  #ifdef CONFIG_X86_64
>  	case VIDIOC_DQEVENT32:
>  		return VIDIOC_DQEVENT;
> +#endif
>  #ifdef CONFIG_COMPAT_32BIT_TIME
>  	case VIDIOC_DQEVENT32_TIME32:
>  		return VIDIOC_DQEVENT;
> -#endif
>  #endif
>  	}
>  	return cmd;
> @@ -1032,10 +1025,10 @@ int v4l2_compat_put_user(void __user *arg, void *parg, unsigned int cmd)
>  #ifdef CONFIG_X86_64
>  	case VIDIOC_DQEVENT32:
>  		return put_v4l2_event32(parg, arg);
> +#endif
>  #ifdef CONFIG_COMPAT_32BIT_TIME
>  	case VIDIOC_DQEVENT32_TIME32:
>  		return put_v4l2_event32_time32(parg, arg);
> -#endif
>  #endif
>  	}
>  	return 0;
>