| Message ID | 20211026055010.1569728-1-arnd@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: v4l2-core: fix VIDIOC_DQEVENT handling on non-x86 | expand |
On Mon, Oct 25, 2021 at 10:50 PM Arnd Bergmann <arnd@kernel.org> wrote: > > From: Arnd Bergmann <arnd@arndb.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@linaro.org> > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Tested-by: John Stultz <john.stultz@linaro.org> Thanks so much again Arnd! -john
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@arndb.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@linaro.org> > Signed-off-by: Arnd Bergmann <arnd@arndb.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; >
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;