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

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

On Fri, Sep 22, 2017 at 8:47 AM, Baolin Wang <baolin.wang@...aro.org> wrote:
> On 21 September 2017 at 20:50, Arnd Bergmann <arnd@...db.de> wrote:
>> On Thu, Sep 21, 2017 at 8:18 AM, Baolin Wang <baolin.wang@...aro.org> wrote:
>>> The struct snd_pcm_sync_ptr will use 'timespec' type variables to record
>>
>> This looks correct, but there is a subtlety here to note about x86-32
>> that we discussed in a previous (private) review. To recall my earlier
>> thoughts:
>>
>> Normal architectures insert 32 bit padding after 'suspended_state',
>> and 32-bit architectures (including x32) also after hw_ptr,
>> but x86-32 does not. You make that explicit in the compat code,
>> this version just relies on the compiler using identical padding
>> in user and kernel space. We could make that explicit using
>>
>> struct snd_pcm_mmap_status64 {
>>        snd_pcm_state_t state;          /* RO: state - SNDRV_PCM_STATE_XXXX */
>>        int pad1;                       /* Needed for 64 bit alignment */
>>        snd_pcm_uframes_t hw_ptr;       /* RO: hw ptr (0...boundary-1) */
>> #if !defined(CONFIG_64BIT) && !defined(CONFIG_X86_32)
>>        int pad2;
>> #endif
>>       struct { s64 tv_sec; s64 tv_nsec; } tstamp;             /* Timestamp */
>>        snd_pcm_state_t suspended_state; /* RO: suspended stream state */
>> #if !defined(CONFIG_X86_32)
>>        int pad3;
>> #endif
>>       struct { s64 tv_sec; s64 tv_nsec; } audio_tstamp;       /* from
>> sample counter or wall clock */
>> };
>
> I am sorry I did not get you here, why we do not need pad2 and pad3
> for x86_32?

This is again the x86-32 alignment quirk: the structure as defined
in the uapi header does not have padding, and the new s64 fields
have 32-bit alignment on x86, so the compiler does not add implicit
padding in user space.

On all other architectures, the fields do get padded implicitly
in user space, I'm just listing the padding explicitly.

> You missed ‘#if !defined(CONFIG_64BIT)“ at the second #if
> condition?

No, that was intentional:

snd_pcm_uframes_t is 'unsigned long', so on 64-bit architectures
we have no padding between two 64-bit values (hw_ptr and tstamp),
and on x86-32 we have no padding because both have 32-bit
alignment.

However, snd_pcm_state_t is 'int', which is always 32-bit wide,
so we do have padding on both 32-bit and 64-bit architectures
between syspended_state and audio_tstamp, with the exception
of x86-32.

      Arnd