linux-kernel - Re: [PATCH v4 6/8] media: v4l2-core: fix v4l2

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 26 Nov 2019 16:24:08 +0100
From:   Hans Verkuil <hverkuil@...all.nl>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Mauro Carvalho Chehab <mchehab@...nel.org>,
        Linux Media Mailing List <linux-media@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        y2038 Mailman List <y2038@...ts.linaro.org>
Subject: Re: [PATCH v4 6/8] media: v4l2-core: fix v4l2_buffer handling for
 time64 ABI

On 11/26/19 4:17 PM, Arnd Bergmann wrote:
> On Tue, Nov 26, 2019 at 3:15 PM Hans Verkuil <hverkuil@...all.nl> wrote:
>>
>> Then use that in the struct v4l2_buffer definition:
>>
>> struct v4l2_buffer {
>> ...
>> #ifdef __KERNEL__
>>         struct __kernel_v4l2_timeval timestamp;
>> #else
>>         struct timeval               timestamp;
>> #endif
>>
>> That keeps struct v4l2_buffer fairly clean. And it also makes it
>> possible to have a bit more extensive documentation for the
>> struct __kernel_v4l2_timeval without polluting the actual struct
>> v4l2_buffer definition.
> 
> Yes, good idea. I've added this version now:
> 
> #ifdef __KERNEL__
> /*
>  * This corresponds to the user space version of timeval
>  * for 64-bit time_t. sparc64 is different from everyone
>  * else, using the microseconds in the wrong half of the
>  * second 64-bit word.
>  */
> struct __kernel_v4l2_timeval {
>         long long       tv_sec;
> #if defined(__sparc__) && defined(__arch64__)
>         int             tv_usec;
>         int             __pad;
> #else
>         long long       tv_usec;
> #endif
> };
> #endif
> 
> I briefly considered using #else #define __kernel_v4l2_timeval timeval
> to avoid the second #ifdef, but went back to your version again
> for clarify.
> 
>> The videodev2.h header is something users of the API look at a
>> lot and having this really ugly kernel timestamp in there is
>> not acceptably IMHO. But splitting it off should work.
> 
> Do you also mean moving it into a separate header file, or
> just outside of struct v4l2_buffer? Since it's hidden in #ifdef
> __KERNEL__, it could be moved to media/ioctl.h or elsewhere.

I've thought about that, but that risks having to change drivers
since they would now have to include another header to get the
right timeval definition. In the end I don't think it is worth the
effort.

I think it is best to define __kernel_v4l2_timeval just before
the struct v4l2_requestbuffers definition rather than before the
struct v4l2_buffer. That way it doesn't interfere with the
userspace structs for the buffer API.

Regards,

	Hans

> 
>       Arnd
>