linux-kernel - Re: [RFC PATCH 2/7] sound: core: Avoid using timespec for struct snd_pcm

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <s5hfubfdn4h.wl-tiwai@suse.de>
Date:   Fri, 22 Sep 2017 12:49:18 +0200
From:   Takashi Iwai <tiwai@...e.de>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Baolin Wang <baolin.wang@...aro.org>,
        Jaroslav Kysela <perex@...ex.cz>, alsa-devel@...a-project.org,
        Arvind Yadav <arvind.yadav.cs@...il.com>,
        Bhumika Goyal <bhumirks@...il.com>,
        Deepa Dinamani <deepa.kernel@...il.com>,
        Liam Girdwood <lgirdwood@...il.com>, dharageswari.r@...el.com,
        gudishax.kranthikumar@...el.com, guneshwor.o.singh@...el.com,
        hardik.t.shah@...el.com, jeeja.kp@...el.com,
        Naveen M <naveen.m@...el.com>,
        Vinod Koul <vinod.koul@...el.com>,
        Mark Brown <broonie@...nel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Dan Carpenter <dan.carpenter@...cle.com>,
        Takashi Sakamoto <o-takashi@...amocchi.jp>,
        Fabian Frederick <fabf@...net.be>,
        SF Markus Elfring <elfring@...rs.sourceforge.net>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH 2/7] sound: core: Avoid using timespec for struct snd_pcm_status

On Fri, 22 Sep 2017 12:14:55 +0200,
Arnd Bergmann wrote:
> 
> On Fri, Sep 22, 2017 at 11:31 AM, Takashi Iwai <tiwai@...e.de> wrote:
> > On Thu, 21 Sep 2017 08:18:04 +0200,
> > Baolin Wang wrote:
> >>
> >> The struct snd_pcm_status will use 'timespec' type variables to record
> >> timestamp, which is not year 2038 safe on 32bits system.
> >>
> >> Userspace will use SNDRV_PCM_IOCTL_STATUS and SNDRV_PCM_IOCTL_STATUS_EXT
> >> as commands to issue ioctl() to fill the 'snd_pcm_status' structure in
> >> userspace. The command number is always defined through _IOR/_IOW/IORW,
> >> so when userspace changes the definition of 'struct timespec' to use
> >> 64-bit types, the command number also changes.
> >>
> >> Thus in the kernel, we now need to define two versions of each such ioctl
> >> and corresponding ioctl commands to handle 32bit time_t and 64bit time_t
> >> in native mode:
> >> struct snd_pcm_status32 {
> >>       ......
> >>       struct { s32 tv_sec; s32 tv_nsec; } trigger_tstamp;
> >>       struct { s32 tv_sec; s32 tv_nsec; } tstamp;
> >>       ......
> >> }
> >>
> >> struct snd_pcm_status64 {
> >>       ......
> >>       struct { s64 tv_sec; s64 tv_nsec; } trigger_tstamp;
> >>       struct { s64 tv_sec; s64 tv_nsec; } tstamp;
> >>       ......
> >> }
> >
> > I'm confused.  It's different from timespec64?  So 32bit user-space
> > would need to use a new own-type timespec instead of the standard
> > timespec that is compliant with y2038?
> 
> It's complicated:
> 
> The definition of 'timespec' that user space sees comes from glibc,
> and while that currently uses the traditional '{ long tv_sec;
> long tv_nsec; }' definition, it will have to change to something like
> (still simplified):
> 
> #if __32BIT && __64_BIT_TIME_T
> typedef long long time_t;
> #else
> typedef long time_t;
> #endif
> struct timespec {
>         time_t tv_sec;
> #if __BIG_ENDIAN && __32BIT && __64_BIT_TIME_T
>         unsigned int :32;
> #endif
>        long tv_nsec;
> #if __LITTLE_ENDIAN && __32BIT && __64_BIT_TIME_T
>         unsigned int pad;
> #endif
> } __attribute__((aligned(8)));
> 
> which matches the layout that a 64-bit kernel uses, aside
> from the nanosecond padding.

Wow, that is really messy.


> The kernel uses timespec64 internally, which is defined as
> "{ s64 tv_sec; long tv_nsec };", so this has the padding
> in a different place on big-endian architectures, and has a
> different alignment and size on i386. We plan to introduce
> a 'struct __kernel_timespec' that is compatible with the
> __64_BIT_TIME_T version of the user timespec, but that
> doesn't exist yet.
> 
> If you prefer, we can probably introduce it now with Baolin's
> series, I think Deepa was planning to post a patch to add
> it soon anyway.

Yes, this sounds like a saner solution than defining the own timespec
at each place individually.  Then we can have better conversion
macros, too, I suppose.

And, if we have kernel_timespec (or kernel_timespec64 or such), can
this deprecate the existing timespec64 usages, too?  I see that
timespec64 is internal only, so consolidation would be beneficial for
code simplification.


Thanks!

Takashi