linux-kernel - Re: [PATCH glibc 5/9] glibc: Perform rseq(2) registration at C startup and thread creation (v17)

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: <87ftcnrf7d.fsf@mid.deneb.enyo.de>
Date:   Tue, 28 Apr 2020 14:02:14 +0200
From:   Florian Weimer <fw@...eb.enyo.de>
To:     Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc:     Michael Kerrisk <mtk.manpages@...il.com>,
        libc-alpha <libc-alpha@...rceware.org>,
        carlos <carlos@...hat.com>, Rich Felker <dalias@...c.org>,
        linux-api <linux-api@...r.kernel.org>,
        Boqun Feng <boqun.feng@...il.com>,
        Will Deacon <will.deacon@....com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Ben Maurer <bmaurer@...com>, Dave Watson <davejwatson@...com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Paul <paulmck@...ux.vnet.ibm.com>, Paul Turner <pjt@...gle.com>,
        Joseph Myers <joseph@...esourcery.com>,
        Szabolcs Nagy <szabolcs.nagy@....com>
Subject: Re: [PATCH glibc 5/9] glibc: Perform rseq(2) registration at C startup and thread creation (v17)

* Mathieu Desnoyers:

>>>>> +/* struct rseq is aligned on 4 * 8 bytes to ensure it is always
>>>>> +   contained within a single cache-line.
>>>>> +
>>>>> +   A single struct rseq per thread is allowed.  */
>>>>> +struct rseq
>>>>> +  {
>>>>> +    /* Restartable sequences cpu_id_start field. Updated by the
>>>>> +       kernel. Read by user-space with single-copy atomicity
>>>>> +       semantics. This field should only be read by the thread which
>>>>> +       registered this data structure. Aligned on 32-bit. Always
>>>> 
>>>> What does “Aligned on 32-bit” mean in this context?  Do you mean to
>>>> reference 32-*byte* alignment here?
>>>
>>> No. I really mean 32-bit (4-byte). Being aligned on 32-byte guarantees that
>>> this field is aligned at least on 4-byte. This is required by single-copy
>>> atomicity semantics.
>>>
>>> Should I update this comment to state "Aligned on 4-byte" instead ?
>> 
>> I think this is implied by all Linux ABIs.  And the explicit alignment
>> specification for struct rseq makes the alignment 32 bytes.
>
> Unless a structure ends up being packed, which is of course not the case
> here.
>
> I would prefer to keep the comment about 32-bit alignment requirement on
> the specific fields, because the motivation for alignment requirement is
> much more strict for fields (correctness) than the motivation for alignment
> of the structure (performance).

But the correctness is already enforced by the compiler, so I fail to
see point of mentioning this in the comment.

Anyway, I don't want to make a big deal of it.  Please leave it in if
you think it is ehlpful.

> x32 should not be an issue as explained above, so I'm very open to
> add this "uptr" for user-space only.

Okay, then please use anonymous unions and structs as necessary, to
ensure that the uptr field can be reached on all platforms in the same
way.