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

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

Message-ID: <CAMz4kuKNrnWSh8TnPbpCv_KWbWTqoXd=2tWUfJ9bQ=4cK_y2zw@mail.gmail.com>
Date:   Wed, 27 Sep 2017 06:24:21 +0800
From:   Baolin Wang <baolin.wang@...aro.org>
To:     Arnd Bergmann <arnd@...db.de>
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 22 September 2017 at 16:48, Arnd Bergmann <arnd@...db.de> wrote:
> 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.

Make sense.

>
>> 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.

Thanks, I can understand now.

-- 
Baolin.wang
Best Regards