[PATCH] sockaddr.3type: Document that sockaddr_storage is the API to be used

[PATCH] sockaddr.3type: Document that sockaddr_storage is the API to be used

[Alejandro Colomar]

POSIX.1 Issue 8 will fix the long-standing issue with sockaddr APIs,
which inevitably caused UB either on user code, libc, or more likely,
both.  sockaddr_storage has been clarified to be implemented in a manner
that aliasing it is safe (suggesting a unnamed union, or other compiler
magic).

Link: <https://www.austingroupbugs.net/view.php?id=1641>
Reported-by: Bastien Roucariès <rouca@debian.org>
Reported-by: Alejandro Colomar <alx@kernel.org>
Cc: glibc <libc-alpha@sourceware.org>
Cc: GCC <gcc@gcc.gnu.org>
Cc: Eric Blake <eblake@redhat.com>
Cc: Stefan Puiu <stefan.puiu@gmail.com>
Cc: Igor Sysoev <igor@sysoev.ru>
Cc: Rich Felker <dalias@libc.org>
Cc: Andrew Clayton <andrew@digital-domain.net>
Cc: Richard Biener <richard.guenther@gmail.com>
Cc: Zack Weinberg <zack@owlfolio.org>
Cc: Florian Weimer <fweimer@redhat.com>
Cc: Joseph Myers <joseph@codesourcery.com>
Cc: Jakub Jelinek <jakub@redhat.com>
Cc: Sam James <sam@gentoo.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---

Hi all,

This is my proposal for documenting the POSIX decission of fixing the
definition of sockaddr_storage.  Bastien, I believe you had something
similar in mind; please review.  Eric, thanks again for the fix!  Could
you please also have a look at this?

Cheers,

Alex

 man3type/sockaddr.3type | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/man3type/sockaddr.3type b/man3type/sockaddr.3type
index 32c3c5bd0..d1db87d5d 100644
--- a/man3type/sockaddr.3type
+++ b/man3type/sockaddr.3type
@@ -23,6 +23,14 @@ .SH SYNOPSIS
 .PP
 .B struct sockaddr_storage {
 .BR "    sa_family_t     ss_family;" "      /* Address family */"
+.PP
+.RS 4
+/* This structure is not really implemented this way.  It may be
+\&   implemented with an unnamed union or some compiler magic to
+\&   avoid breaking aliasing rules when accessed as any other of the
+\&   sockaddr_* structures documented in this page.  See CAVEATS.
+\& */
+.RE
 .B };
 .PP
 .BR typedef " /* ... */ " socklen_t;
@@ -122,6 +130,20 @@ .SH NOTES
 .I <netinet/in.h>
 and
 .IR <sys/un.h> .
+.SH CAVEATS
+To avoid breaking aliasing rules,
+programs that use functions that receive pointers to
+.I sockaddr
+structures should declare objects of type
+.IR sockaddr_storage ,
+which is defined in a way that it
+can be accessed as any of the different structures defined in this page.
+Failure to do so may result in Undefined Behavior.
+.PP
+New functions should be written to accept pointers to
+.I sockaddr_storage
+instead of the traditional
+.IR sockaddr .
 .SH SEE ALSO
 .BR accept (2),
 .BR bind (2),
-- 
2.39.2

Re: [PATCH] sockaddr.3type: Document that sockaddr_storage is the API to be used

[Eric Blake]

On Thu, Mar 30, 2023 at 07:13:11PM +0200, Alejandro Colomar wrote:
> POSIX.1 Issue 8 will fix the long-standing issue with sockaddr APIs,
> which inevitably caused UB either on user code, libc, or more likely,
> both.  sockaddr_storage has been clarified to be implemented in a manner
> that aliasing it is safe (suggesting a unnamed union, or other compiler
> magic).
> 
> Link: <https://www.austingroupbugs.net/view.php?id=1641>
> Reported-by: Bastien Roucariès <rouca@debian.org>
> Reported-by: Alejandro Colomar <alx@kernel.org>
> Cc: glibc <libc-alpha@sourceware.org>
> Cc: GCC <gcc@gcc.gnu.org>
> Cc: Eric Blake <eblake@redhat.com>
> Cc: Stefan Puiu <stefan.puiu@gmail.com>
> Cc: Igor Sysoev <igor@sysoev.ru>
> Cc: Rich Felker <dalias@libc.org>
> Cc: Andrew Clayton <andrew@digital-domain.net>
> Cc: Richard Biener <richard.guenther@gmail.com>
> Cc: Zack Weinberg <zack@owlfolio.org>
> Cc: Florian Weimer <fweimer@redhat.com>
> Cc: Joseph Myers <joseph@codesourcery.com>
> Cc: Jakub Jelinek <jakub@redhat.com>
> Cc: Sam James <sam@gentoo.org>
> Signed-off-by: Alejandro Colomar <alx@kernel.org>
> ---
> 
> Hi all,
> 
> This is my proposal for documenting the POSIX decission of fixing the
> definition of sockaddr_storage.  Bastien, I believe you had something
> similar in mind; please review.  Eric, thanks again for the fix!  Could
> you please also have a look at this?
> 
> Cheers,
> 
> Alex
> 
>  man3type/sockaddr.3type | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/man3type/sockaddr.3type b/man3type/sockaddr.3type
> index 32c3c5bd0..d1db87d5d 100644
> --- a/man3type/sockaddr.3type
> +++ b/man3type/sockaddr.3type
> @@ -23,6 +23,14 @@ .SH SYNOPSIS
>  .PP
>  .B struct sockaddr_storage {
>  .BR "    sa_family_t     ss_family;" "      /* Address family */"
> +.PP
> +.RS 4
> +/* This structure is not really implemented this way.  It may be
> +\&   implemented with an unnamed union or some compiler magic to
> +\&   avoid breaking aliasing rules when accessed as any other of the
> +\&   sockaddr_* structures documented in this page.  See CAVEATS.
> +\& */

Do we want similar comments in struct sockaddr and/or sockaddr_XX?

> +.RE
>  .B };
>  .PP
>  .BR typedef " /* ... */ " socklen_t;
> @@ -122,6 +130,20 @@ .SH NOTES
>  .I <netinet/in.h>
>  and
>  .IR <sys/un.h> .
> +.SH CAVEATS
> +To avoid breaking aliasing rules,
> +programs that use functions that receive pointers to
> +.I sockaddr
> +structures should declare objects of type
> +.IR sockaddr_storage ,
> +which is defined in a way that it
> +can be accessed as any of the different structures defined in this page.
> +Failure to do so may result in Undefined Behavior.

Existing POSIX already requires sockaddr_storage to be suitably sized
and aligned to overlay with all other sockaddr* types.  What the
recent POSIX bug change does is add wording to emphasize that casts in
any of the 6 directions:

sockaddr* <-> sockaddr_XX*
sockaddr_storage* <-> sockaddr*
sockaddr_storage* <-> sockaddr_XX*

must allow the sa_family/ss_family/sa_family_t member to overlay
without triggering undefined behavior due to bad aliasing, at which
point, access to that member lets you deduce what other object type
you really have.  But you are also correct that merely casting a
pointer to another larger struct that doesn't trigger aliasing, but
then dereferencing beyond the bounds of the original, is not intended
to be portable.  The aliasing diagnostics are suppressed because of
the requirements on the first member, so now the user must now be
careful that their access of remaining members is safe even if the
compiler is no longer helping them because of the magic that
suppressed the aliasing detection.

I agree with your warning that code that can handle generic socket
types should use sockaddr_storage (and not sockaddr) as the original
object (the one object that the standard requires to be suitably sized
and aligned to overlay with the entirety of all other sockaddr types,
rather than just the sa_family_t first member), although we may want
to be more precise that code using a specific protocol type can
directly use the proper sockaddr_XX type rather than having to use an
intermediate sockaddr_storage.

I'm not sure if there are better ways to word that paragraph to convey
the intended sentiment.

> +.PP
> +New functions should be written to accept pointers to
> +.I sockaddr_storage
> +instead of the traditional
> +.IR sockaddr .

I'm less certain about this one.  The POSIX wording specifically chose
to keep existing API/ABI of sockaddr* in all the standardized
functions unchanged, as it would be too invasive to existing code to
change the signatures now.  The burden is on the system headers to
define types so that the necessary casts (present in lots of existing
code because sockaddr* has a bit more type-safety than void*) do not
of themselves cause aliasing issues, and therefore avoid undefined
behavior provided subsequent code accessing through the pointers is
not accessing beyond the bounds of the real object.  The likelihood of
POSIX adding new socket APIs taking sockaddr_storage* just to enforce
non-aliasing seems slim.  But then again, this advice applies to more
than just functions likely to be standardized in a future libc, so
maybe this paragraph is worth it after all.

>  .SH SEE ALSO
>  .BR accept (2),
>  .BR bind (2),
> -- 
> 2.39.2
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH] sockaddr.3type: Document that sockaddr_storage is the API to be used

[Alejandro Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 16369 bytes --]

Hi Eric,

I'm going to reply both your emails here so that GCC is CCed, and they can
suggest better stuff.  I'm worried about sending something to POSIX without
enough eyes checking it.  So this will be a long email.


On 3/30/23 20:36, eblake wrote:
> On Thu, Mar 30, 2023 at 06:25:30PM +0200, Alejandro Colomar wrote:
>> Hi Eric!
>>
>> On 3/30/23 17:22, Austin Group Bug Tracker via austin-group-l at The Open Group wrote:
>>>
>>> The following issue has been RESOLVED. 
>>> ====================================================================== 
>>> https://austingroupbugs.net/view.php?id=1641
> ...
>>
>> Thanks for taking care of this bug!
> 
> My pleasure.

:-)

> 
>>
>>
>> On 3/30/23 17:20, Austin Group Bug Tracker via austin-group-l at The Open Group wrote:
>>> On page 386 line 13115 section <sys/socket.h> DESCRIPTION, change:
>>>
>>>     When a pointer to a sockaddr_storage structure is cast as a pointer to a sockaddr structure, the ss_family field of the sockaddr_storage structure shall map onto the sa_family field of the sockaddr structure. When a pointer to a sockaddr_storage structure is cast as a pointer to a protocol-specific address structure, the ss_family field shall map onto a field of that structure that is of type sa_family_t and that identifies the protocol’s address family.
>>>
>>> to:
>>>
>>>     When a pointer to a sockaddr_storage structure is cast as a pointer to a sockaddr structure, or vice versa, the ss_family field of the sockaddr_storage structure shall map onto the sa_family field of the sockaddr structure. When a pointer to a sockaddr_storage structure is cast as a pointer to a protocol-specific address structure, or vice versa, the ss_family field shall map onto a field of that structure that is of type sa_family_t and that identifies the protocol’s address family. When a pointer to a sockaddr structure is cast as a pointer to a protocol-specific address structure, or vice versa, the sa_family field shall map onto a field of that structure that is of type sa_family_t and that identifies the protocol’s address family. Additionally, the structures shall be defined in such a way that these casts do not cause the compiler to produce diagnostics about aliasing issues when compiling conforming application (xref to XBD section 2.2) source files.
>>
>> I will add a CAVEATS section in sockaddr(3type) covering this, and will
>> CC you just to check.
> 
> Sure, I'll be happy to review.
> 
> The intent from the meeting (and perhaps requires a bit of reading
> between the lines compared to what was captured as the approved text)
> was that implementations MUST ensure that existing code does not get
> miscompiled under the guise of undefined behavior, but without stating
> how to do it other than suggesting that implementation-specific
> extensions may be needed (somewhat similar as how POSIX requires that
> when dlsym() returns a void* for a function entry point, converting
> that pointer to a function pointer that can then be called MUST be
> compiled to work as intended, even though C doesn't define it).  We
> want the burden to be on the libc and system header providers to
> guarantee defined behavior, and not on the average coder to make
> careful use of memcpy() between storage of different effective types
> to avoid what might be otherwise undefined if it were written using
> types declared using only C99 syntax.
> 
> Whether gcc already has all the attributes you need is not my area of
> expertise.  In my skim of the glibc list conversation, I saw mention
> of attribute [[gnu:transparent_union]] rather than [[__may_alias__]] -
> if that's a better implementation-defined extension that does what we
> need, then use it.  The standard developers were a bit uncomfortable
> directly putting [[gnu:transparent_union]] in the standard, but
> [[__may_alias__]] was noncontroversial (it's in the namespace reserved
> for the implementation)

Not really; implementation-defined attributes are required to use an
implementation-defined prefix like 'gnu::'.  So [[__may_alias__]] is
reserved by ISO C, AFAIR.  Maybe it would be better to just mention
attributes without any specific attribute name; being fuzzy about it
would help avoid making promises that we can't hold.

> and deemed to be a sufficient hint for
> developers to figure out that they can use whatever works best to meet
> the actual requirement of not letting the compiler optimize away
> socket operations under the premise of undefined behavior.
> 
>>>
>>> On page 390 line 13260 section <sys/socket.h> APPLICATION USAGE, append a sentence:
>>>
>>>     Note that this example only deals with size and alignment; see RATIONALE for additional issues related to these structures.
>>>
>>>
>>>
>>> On page 390 line 13291 section <sys/socket.h>, change RATIONALE from "None" to:
>>>
>>>     Note that defining the sockaddr_storage and sockaddr structures using only mechanisms defined in editions of the ISO C standard prior to the 2011 edition (C11) may produce aliasing diagnostics when used in C11 and later editions of the ISO C standard. Because of the large body of existing code utilizing sockets in a way that was well-defined in the 1999 edition of the ISO C standard (C99) but could trigger undefined behavior if C11/C17 aliasing detection were enforced, this standard mandates that casts between pointers to the various socket address structures do not produce aliasing diagnostics, so as to preserve well-defined semantics. An implementation's header files may need to use anonymous unions, or even an implementation-specific extension such as a <tt>[[__may_alias__]]</tt> attribute, to comply with the requirements of this standard.
>>
>>
>> I'm not sure how aliasing rules changed from C99 to C11.  Wasn't
>> aliasing already in C99 (and also in C89)?  I believe this was
>> covered by 6.5.7, which is the same in both C99 and C11.
>>
>> <https://port70.net/~nsz/c/c11/n1570.html#6.5p7>
>> <https://port70.net/~nsz/c/c99/n1256.html#6.5p7>
> 
> I'm also not sure about where the requirements started making things
> undefined behavior.  I do recall Nick Stoughton mentioning that he
> seems to remember 'restrict' behavior changing between C99 and C11, so
> maybe that's why he assumed that C99 permits the behavior without
> undefined behvaior; but reading
> https://port70.net/~nsz/c/c11/n1570.html#Foreword I don't see anything
> along those lines in C11 that wasn't in C99.  It does mention that
> anonymous unions are new to C11; the Austin Group was unsure whether

Oh, I just learned that anonymous unions are in C11.  I thought they
were GNU extensions.  Nice to know.

> anonymous unions are sufficent to solve the problem on their own
> (without also causing namespace pollution issues), or if an
> implementation-defined extension is needed.  And maybe compilers got
> better at alias detection because of the introduction of anonymous
> unions.

I'm not sure either.

> 
> At any rate, since you did demonstrate that the C11 and C99 wording is
> essentially the same, I'm happy to forward any alternative wording
> corrections you propose, and I can bring the topic back up in next
> week's meeting (if we decide that C99 indeed has undefined behavior,
> rather than our assumption that it wasn't undefined until C11, we may
> need to issue an interpretation against Issue 7, rather than just
> tweaking the wording for Issue 8 when we swap over to C17 as the
> mandated baseline).

I would just make it more fuzzy about which standard version did what.
How about this?:

[[
Note that defining the sockaddr_storage and sockaddr structures using
only mechanisms defined in editions of the ISO C standard may produce
aliasing diagnostics.  Because of the large body of existing code
utilizing sockets in a way that could trigger undefined behavior due
to strict aliasing rules, this standard mandates that the various socket
address structures can alias each other for accessing their first member,
so as to preserve well-defined semantics.  An implementation's header
files may need to use anonymous unions, or even an
implementation-specific extension to comply with the requirements of
this standard.
]]

> 
> And since both C99 and C11 state that accessing the stored value of an
> object is permissible through
> 
> "a type compatible with the effective type of the object,"
> 
> it seems like the obvious action in glibc is to do whatever it takes
> to convince the compiler that struct sockaddr, struct
> sockaddr_storage, and all of the individual sockaddr_XX protocol types
> are marked with whatever magic that lets the compiler know that they
> are compatible types (not necessarily according to the C rules of
> compatible types, but according to the rules of the extension
> attribute).  I don't know if glibc can do it in isolation, or if gcc
> will need to invent yet another compiler attribute for glibc's use.
> 
-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


On 3/30/23 21:11, Eric Blake wrote:
> On Thu, Mar 30, 2023 at 07:13:11PM +0200, Alejandro Colomar wrote:
>> POSIX.1 Issue 8 will fix the long-standing issue with sockaddr APIs,
>> which inevitably caused UB either on user code, libc, or more likely,
>> both.  sockaddr_storage has been clarified to be implemented in a manner
>> that aliasing it is safe (suggesting a unnamed union, or other compiler
>> magic).
>>
>> Link: <https://www.austingroupbugs.net/view.php?id=1641>
>> Reported-by: Bastien Roucariès <rouca@debian.org>
>> Reported-by: Alejandro Colomar <alx@kernel.org>
>> Cc: glibc <libc-alpha@sourceware.org>
>> Cc: GCC <gcc@gcc.gnu.org>
>> Cc: Eric Blake <eblake@redhat.com>
>> Cc: Stefan Puiu <stefan.puiu@gmail.com>
>> Cc: Igor Sysoev <igor@sysoev.ru>
>> Cc: Rich Felker <dalias@libc.org>
>> Cc: Andrew Clayton <andrew@digital-domain.net>
>> Cc: Richard Biener <richard.guenther@gmail.com>
>> Cc: Zack Weinberg <zack@owlfolio.org>
>> Cc: Florian Weimer <fweimer@redhat.com>
>> Cc: Joseph Myers <joseph@codesourcery.com>
>> Cc: Jakub Jelinek <jakub@redhat.com>
>> Cc: Sam James <sam@gentoo.org>
>> Signed-off-by: Alejandro Colomar <alx@kernel.org>
>> ---
>>
>> Hi all,
>>
>> This is my proposal for documenting the POSIX decission of fixing the
>> definition of sockaddr_storage.  Bastien, I believe you had something
>> similar in mind; please review.  Eric, thanks again for the fix!  Could
>> you please also have a look at this?
>>
>> Cheers,
>>
>> Alex
>>
>>  man3type/sockaddr.3type | 22 ++++++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>>
>> diff --git a/man3type/sockaddr.3type b/man3type/sockaddr.3type
>> index 32c3c5bd0..d1db87d5d 100644
>> --- a/man3type/sockaddr.3type
>> +++ b/man3type/sockaddr.3type
>> @@ -23,6 +23,14 @@ .SH SYNOPSIS
>>  .PP
>>  .B struct sockaddr_storage {
>>  .BR "    sa_family_t     ss_family;" "      /* Address family */"
>> +.PP
>> +.RS 4
>> +/* This structure is not really implemented this way.  It may be
>> +\&   implemented with an unnamed union or some compiler magic to
>> +\&   avoid breaking aliasing rules when accessed as any other of the
>> +\&   sockaddr_* structures documented in this page.  See CAVEATS.
>> +\& */
> 
> Do we want similar comments in struct sockaddr and/or sockaddr_XX?

I had understood the POSIX change differently.  Considering your
explanation below, I'll move this to a general NOTES section that
applies to all structures.

> 
>> +.RE
>>  .B };
>>  .PP
>>  .BR typedef " /* ... */ " socklen_t;
>> @@ -122,6 +130,20 @@ .SH NOTES
>>  .I <netinet/in.h>
>>  and
>>  .IR <sys/un.h> .
>> +.SH CAVEATS
>> +To avoid breaking aliasing rules,
>> +programs that use functions that receive pointers to
>> +.I sockaddr
>> +structures should declare objects of type
>> +.IR sockaddr_storage ,
>> +which is defined in a way that it
>> +can be accessed as any of the different structures defined in this page.
>> +Failure to do so may result in Undefined Behavior.
> 
> Existing POSIX already requires sockaddr_storage to be suitably sized
> and aligned to overlay with all other sockaddr* types.

Alignment allows the pointers to be converted to each other and back,
but for being able to dereference (access) the pointer, you need stronger
guarantees; basically a veto from the standard to strict aliasing rules,
as the new wording does.

>  What the
> recent POSIX bug change does is add wording to emphasize that casts in
> any of the 6 directions:
> 
> sockaddr* <-> sockaddr_XX*
> sockaddr_storage* <-> sockaddr*
> sockaddr_storage* <-> sockaddr_XX*

Hmm, okay.  I guess this still doesn't intend to allow traveling
this way:

sockaddr_XX <-> sockaddr_storage <-> sockaddr_YY

Not even for checking the first common member, right?  Just to
be sure of the intention.

> 
> must allow the sa_family/ss_family/sa_family_t member to overlay
> without triggering undefined behavior due to bad aliasing, at which
> point, access to that member lets you deduce what other object type
> you really have.  But you are also correct that merely casting a
> pointer to another larger struct that doesn't trigger aliasing, but
> then dereferencing beyond the bounds of the original, is not intended
> to be portable.  The aliasing diagnostics are suppressed because of
> the requirements on the first member, so now the user must now be
> careful that their access of remaining members is safe even if the
> compiler is no longer helping them because of the magic that
> suppressed the aliasing detection.

Yep, I was worried about completely disabling aliasing rules for
this reason: we're effectively telling compilers and static analyzers
that this is not the droid they're looking for.  Hopefully, checking
the first field is so simple that no program should forget to do it.

If there's fear that this is a problem, we could maybe allow a smaller
set of conversions.

> 
> I agree with your warning that code that can handle generic socket
> types should use sockaddr_storage (and not sockaddr) as the original
> object (the one object that the standard requires to be suitably sized
> and aligned to overlay with the entirety of all other sockaddr types,
> rather than just the sa_family_t first member), although we may want
> to be more precise that code using a specific protocol type can
> directly use the proper sockaddr_XX type rather than having to use an
> intermediate sockaddr_storage.

It seems reasonable, since it might be an unacceptable overhead if you
declare many such objects, and want your program to stay low on memory.
Using the smallest structure possible may be important to some programs.

> 
> I'm not sure if there are better ways to word that paragraph to convey
> the intended sentiment.
> 
>> +.PP
>> +New functions should be written to accept pointers to
>> +.I sockaddr_storage
>> +instead of the traditional
>> +.IR sockaddr .
> 
> I'm less certain about this one.  The POSIX wording specifically chose
> to keep existing API/ABI of sockaddr* in all the standardized
> functions unchanged, as it would be too invasive to existing code to
> change the signatures now.  The burden is on the system headers to
> define types so that the necessary casts (present in lots of existing
> code because sockaddr* has a bit more type-safety than void*) do not
> of themselves cause aliasing issues, and therefore avoid undefined
> behavior provided subsequent code accessing through the pointers is
> not accessing beyond the bounds of the real object.  The likelihood of
> POSIX adding new socket APIs taking sockaddr_storage* just to enforce
> non-aliasing seems slim.  But then again, this advice applies to more
> than just functions likely to be standardized in a future libc, so
> maybe this paragraph is worth it after all.

Depending on your answers to my answers above, I think I agree with you
on this.

Cheers,
Alex

> 
>>  .SH SEE ALSO
>>  .BR accept (2),
>>  .BR bind (2),
>> -- 
>> 2.39.2
>>
> 

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] sockaddr.3type: Document that sockaddr_storage is the API to be used

[Eric Blake]

On Wed, Apr 05, 2023 at 02:42:04AM +0200, Alejandro Colomar wrote:
> Hi Eric,
> 
> I'm going to reply both your emails here so that GCC is CCed, and they can
> suggest better stuff.  I'm worried about sending something to POSIX without
> enough eyes checking it.  So this will be a long email.

Because your mail landed in a publicly archived mailing list, the
POSIX folks saw it anyways ;)

...
> > 
> > Whether gcc already has all the attributes you need is not my area of
> > expertise.  In my skim of the glibc list conversation, I saw mention
> > of attribute [[gnu:transparent_union]] rather than [[__may_alias__]] -
> > if that's a better implementation-defined extension that does what we
> > need, then use it.  The standard developers were a bit uncomfortable
> > directly putting [[gnu:transparent_union]] in the standard, but
> > [[__may_alias__]] was noncontroversial (it's in the namespace reserved
> > for the implementation)
> 
> Not really; implementation-defined attributes are required to use an
> implementation-defined prefix like 'gnu::'.  So [[__may_alias__]] is
> reserved by ISO C, AFAIR.  Maybe it would be better to just mention
> attributes without any specific attribute name; being fuzzy about it
> would help avoid making promises that we can't hold.

On this point, the group agreed, and we intentionally loosened to
wording to just mention an implementation-defined extension, rather
than giving any specific attribute name.

...
> 
> I would just make it more fuzzy about which standard version did what.
> How about this?:
> 
> [[
> Note that defining the sockaddr_storage and sockaddr structures using
> only mechanisms defined in editions of the ISO C standard may produce
> aliasing diagnostics.  Because of the large body of existing code
> utilizing sockets in a way that could trigger undefined behavior due
> to strict aliasing rules, this standard mandates that the various socket
> address structures can alias each other for accessing their first member,

The sa_family_t member is not necessarily the first member on all
platforms (it happens to be first in Linux, but as a counter-example,
https://man.freebsd.org/cgi/man.cgi?query=unix&sektion=4 shows
sun_family as the second one-byte field in struct sockaddr_un).  The
emphasis is on derefencing the family member (whatever offset it is
at) to learn what cast to use to then safely access the rest of the
storage.

As such, here's the updated wording that the Austin Group tried today
(and we plan on starting a 30-day interpretation feedback window if
there are still adjustments to be made to the POSIX wording):

https://austingroupbugs.net/view.php?id=1641#c6255

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH] sockaddr.3type: Document that sockaddr_storage is the API to be used

[Alejandro Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 3021 bytes --]

Hi Eric,

On 4/6/23 18:24, Eric Blake wrote:
> On Wed, Apr 05, 2023 at 02:42:04AM +0200, Alejandro Colomar wrote:
>> Hi Eric,
>>
>> I'm going to reply both your emails here so that GCC is CCed, and they can
>> suggest better stuff.  I'm worried about sending something to POSIX without
>> enough eyes checking it.  So this will be a long email.
> 
> Because your mail landed in a publicly archived mailing list, the
> POSIX folks saw it anyways ;)

:)

> 
> ...
>>>
>>> Whether gcc already has all the attributes you need is not my area of
>>> expertise.  In my skim of the glibc list conversation, I saw mention
>>> of attribute [[gnu:transparent_union]] rather than [[__may_alias__]] -
>>> if that's a better implementation-defined extension that does what we
>>> need, then use it.  The standard developers were a bit uncomfortable
>>> directly putting [[gnu:transparent_union]] in the standard, but
>>> [[__may_alias__]] was noncontroversial (it's in the namespace reserved
>>> for the implementation)
>>
>> Not really; implementation-defined attributes are required to use an
>> implementation-defined prefix like 'gnu::'.  So [[__may_alias__]] is
>> reserved by ISO C, AFAIR.  Maybe it would be better to just mention
>> attributes without any specific attribute name; being fuzzy about it
>> would help avoid making promises that we can't hold.
> 
> On this point, the group agreed, and we intentionally loosened to
> wording to just mention an implementation-defined extension, rather
> than giving any specific attribute name.
> 
> ...
>>
>> I would just make it more fuzzy about which standard version did what.
>> How about this?:
>>
>> [[
>> Note that defining the sockaddr_storage and sockaddr structures using
>> only mechanisms defined in editions of the ISO C standard may produce
>> aliasing diagnostics.  Because of the large body of existing code
>> utilizing sockets in a way that could trigger undefined behavior due
>> to strict aliasing rules, this standard mandates that the various socket
>> address structures can alias each other for accessing their first member,
> 
> The sa_family_t member is not necessarily the first member on all
> platforms (it happens to be first in Linux, but as a counter-example,
> https://man.freebsd.org/cgi/man.cgi?query=unix&sektion=4 shows
> sun_family as the second one-byte field in struct sockaddr_un).  The
> emphasis is on derefencing the family member (whatever offset it is
> at) to learn what cast to use to then safely access the rest of the
> storage.
> 
> As such, here's the updated wording that the Austin Group tried today
> (and we plan on starting a 30-day interpretation feedback window if
> there are still adjustments to be made to the POSIX wording):
> 
> https://austingroupbugs.net/view.php?id=1641#c6255

Thanks!  That wording (both paragraphs) LGTM.

Cheers,
Alex

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] sockaddr.3type: Document that sockaddr_storage is the API to be used

[Zack Weinberg]

On Thu, Apr 6, 2023, at 12:31 PM, Alejandro Colomar via Libc-alpha wrote:
> On 4/6/23 18:24, Eric Blake wrote:
>> here's the updated wording that the Austin Group tried today (and we
>> plan on starting a 30-day interpretation feedback window if there are
>> still adjustments to be made to the POSIX wording):
>>
>> https://austingroupbugs.net/view.php?id=1641#c6255
>
> Thanks!  That wording (both paragraphs) LGTM.

If I could suggest an additional change, the focus on aliasing
_diagnostics_ rather misses the point IMHO.  We don't just want the
compiler to _not complain_ about accesses to sa_family_t, we want it to
treat the accesses as _legitimate_.  So, instead of

# Additionally, the structures shall be defined in such a way that
# these casts do not cause the compiler to produce diagnostics about
# aliasing issues in accessing the sa_family_t member of these
# structures when compiling conforming application (xref to XBD section
# 2.2) source files.

may I suggest wording along the lines of

# Additionally, the structures shall be defined in such a way that
# the compiler treats an access to the stored value of the sa_family_t
# member of any of these structures, via an lvalue expression whose type
# involves any other one of these structures, as permissible, despite the
# more restrictive rules listed in ISO C section 6.5p7.

zw

Re: [PATCH] sockaddr.3type: Document that sockaddr_storage is the API to be used

[Eric Blake]

On Thu, Apr 06, 2023 at 02:05:15PM -0400, Zack Weinberg wrote:
> On Thu, Apr 6, 2023, at 12:31 PM, Alejandro Colomar via Libc-alpha wrote:
> > On 4/6/23 18:24, Eric Blake wrote:
> >> here's the updated wording that the Austin Group tried today (and we
> >> plan on starting a 30-day interpretation feedback window if there are
> >> still adjustments to be made to the POSIX wording):
> >>
> >> https://austingroupbugs.net/view.php?id=1641#c6255
> >
> > Thanks!  That wording (both paragraphs) LGTM.
> 
> If I could suggest an additional change, the focus on aliasing
> _diagnostics_ rather misses the point IMHO.  We don't just want the
> compiler to _not complain_ about accesses to sa_family_t, we want it to
> treat the accesses as _legitimate_.  So, instead of
> 
> # Additionally, the structures shall be defined in such a way that
> # these casts do not cause the compiler to produce diagnostics about
> # aliasing issues in accessing the sa_family_t member of these
> # structures when compiling conforming application (xref to XBD section
> # 2.2) source files.
> 
> may I suggest wording along the lines of
> 
> # Additionally, the structures shall be defined in such a way that
> # the compiler treats an access to the stored value of the sa_family_t
> # member of any of these structures, via an lvalue expression whose type
> # involves any other one of these structures, as permissible, despite the
> # more restrictive rules listed in ISO C section 6.5p7.

I like it as an improvement; I've added your suggestion to the POSIX
bug report as one of the comments received during the 30-day
interpretation window, to see what the other standards developers
think.

Since Issue 7 is tied to C99, and Issue 8 will be tied to C17, both of
which use the same section number despite being a different edition of
the C standard, being that specific may work.  Or, we might try
something focusing more on wording instead of document location, as
in:

Additionally, the structures shall be defined in such a way that the
compiler treats an access to the stored value of the sa_family_t
member of any of these structures, via an lvalue expression whose type
involves any other one of these structures, as permissible even if the
types involved would not otherwise be deemed compatible with the
effective type of the object ultimately being accessed.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH] sockaddr.3type: Document that sockaddr_storage is the API to be used

[Zack Weinberg]

On 2023-04-06 3:37 PM, Eric Blake wrote:
> Since Issue 7 is tied to C99, and Issue 8 will be tied to C17, both of
> which use the same section number despite being a different edition of
> the C standard, being that specific may work.  Or, we might try
> something focusing more on wording instead of document location, as
> in:
> 
> Additionally, the structures shall be defined in such a way that the
> compiler treats an access to the stored value of the sa_family_t
> member of any of these structures, via an lvalue expression whose type
> involves any other one of these structures, as permissible even if the
> types involved would not otherwise be deemed compatible with the
> effective type of the object ultimately being accessed.

I quite like this way of putting it.  It subsumes both what I wrote and 
the related potential headache with deciding whether the sa_family_t 
field is considered an object or just a range of bytes within a larger 
object.

zw

Re: [PATCH] sockaddr.3type: Document that sockaddr_storage is the API to be used

[Alejandro Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 2085 bytes --]

Hi Eric,

On 4/14/23 18:08, Zack Weinberg via Libc-alpha wrote:
> On 2023-04-06 3:37 PM, Eric Blake wrote:
>> Since Issue 7 is tied to C99, and Issue 8 will be tied to C17, both of
>> which use the same section number despite being a different edition of
>> the C standard, being that specific may work.  Or, we might try
>> something focusing more on wording instead of document location, as
>> in:
>>
>> Additionally, the structures shall be defined in such a way that the
>> compiler treats an access to the stored value of the sa_family_t
>> member of any of these structures, via an lvalue expression whose type
>> involves any other one of these structures, as permissible even if the
>> types involved would not otherwise be deemed compatible with the
>> effective type of the object ultimately being accessed.

The wording I see in <https://austingroupbugs.net/view.php?id=1641#c6262>
doesn't seem to cover the case of aliasing a sockaddr_storage as a
protocol-specific address for setting other members.

Aliasing rules don't allow one to declare an object of type
sockaddr_storage and then fill the structure as if it were another
structure, even if alignment and size are correct.  We would need
some wording that says something like:

When a pointer to a sockaddr_storage structure is first aliased as a
pointer to a protocol-specific address structure, the effective type
of the object will be set to the protocol-specific structure.

This is similar to what happens when malloc(3) is assigned to a
non-character type.  That's a big hammer, but it does the job.  Maybe
we would need some looser language?  I CCd GCC, in case they have
concerns about this wording.

Cheers,
Alex

> 
> I quite like this way of putting it.  It subsumes both what I wrote and 
> the related potential headache with deciding whether the sa_family_t 
> field is considered an object or just a range of bytes within a larger 
> object.
> 
> zw

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] sockaddr.3type: Document that sockaddr_storage is the API to be used

[Alejandro Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 3005 bytes --]



On 4/21/23 16:58, Alejandro Colomar wrote:
> Hi Eric,
> 
> On 4/14/23 18:08, Zack Weinberg via Libc-alpha wrote:
>> On 2023-04-06 3:37 PM, Eric Blake wrote:
>>> Since Issue 7 is tied to C99, and Issue 8 will be tied to C17, both of
>>> which use the same section number despite being a different edition of
>>> the C standard, being that specific may work.  Or, we might try
>>> something focusing more on wording instead of document location, as
>>> in:
>>>
>>> Additionally, the structures shall be defined in such a way that the
>>> compiler treats an access to the stored value of the sa_family_t
>>> member of any of these structures, via an lvalue expression whose type
>>> involves any other one of these structures, as permissible even if the
>>> types involved would not otherwise be deemed compatible with the
>>> effective type of the object ultimately being accessed.
> 
> The wording I see in <https://austingroupbugs.net/view.php?id=1641#c6262>
> doesn't seem to cover the case of aliasing a sockaddr_storage as a
> protocol-specific address for setting other members.
> 
> Aliasing rules don't allow one to declare an object of type
> sockaddr_storage and then fill the structure as if it were another
> structure, even if alignment and size are correct.  We would need
> some wording that says something like:
> 
> When a pointer to a sockaddr_storage structure is first aliased as a
> pointer to a protocol-specific address structure, the effective type
> of the object will be set to the protocol-specific structure.
> 
> This is similar to what happens when malloc(3) is assigned to a
> non-character type.  That's a big hammer, but it does the job.  Maybe
> we would need some looser language?  I CCd GCC, in case they have
> concerns about this wording.
> 
> Cheers,
> Alex
> 
>>
>> I quite like this way of putting it.  It subsumes both what I wrote and 
>> the related potential headache with deciding whether the sa_family_t 
>> field is considered an object or just a range of bytes within a larger 
>> object.
>>
>> zw
> 

For the man pages, I've rewritten it to the following:


$ git diff
diff --git a/man3type/sockaddr.3type b/man3type/sockaddr.3type
index 2fdf56c59..e610aa0f5 100644
--- a/man3type/sockaddr.3type
+++ b/man3type/sockaddr.3type
@@ -117,6 +117,14 @@ .SH HISTORY
 was invented by POSIX.
 See also
 .BR accept (2).
+.PP
+These structures were invented before modern ISO C strict-aliasing rules.
+If aliasing rules are applied strictly,
+these structures would be impossible to use
+without invoking Undefined Behavior (UB).
+POSIX Issue 8 will fix this by requiring that implementations
+make sure that these structures
+can be safely used as they were designed.
 .SH NOTES
 .I socklen_t
 is also defined in


I guess this is simple enough that it should work as documentation.

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] sockaddr.3type: Document that sockaddr_storage is the API to be used

[Eric Blake]

On Fri, Apr 21, 2023 at 05:00:14PM +0200, Alejandro Colomar wrote:
> > 
> > The wording I see in <https://austingroupbugs.net/view.php?id=1641#c6262>
> > doesn't seem to cover the case of aliasing a sockaddr_storage as a
> > protocol-specific address for setting other members.
> > 
> > Aliasing rules don't allow one to declare an object of type
> > sockaddr_storage and then fill the structure as if it were another
> > structure, even if alignment and size are correct.  We would need
> > some wording that says something like:
> > 
> > When a pointer to a sockaddr_storage structure is first aliased as a
> > pointer to a protocol-specific address structure, the effective type
> > of the object will be set to the protocol-specific structure.

I'll add that as a comment to the Austin Group page; it seems like a
reasonable statement of intent (POSIX already says that struct
sockaddr_storage is sufficiently sized and aligned; all that remains
is for the compiler to be aware that we intend to use a
more-appropriate effective type once we have the storage allocated).

> > 
> > This is similar to what happens when malloc(3) is assigned to a
> > non-character type.  That's a big hammer, but it does the job.  Maybe
> > we would need some looser language?  I CCd GCC, in case they have
> > concerns about this wording.
> > 
> > Cheers,
> > Alex
> > 
> >>
> >> I quite like this way of putting it.  It subsumes both what I wrote and 
> >> the related potential headache with deciding whether the sa_family_t 
> >> field is considered an object or just a range of bytes within a larger 
> >> object.
> >>
> >> zw
> > 
> 
> For the man pages, I've rewritten it to the following:
> 
> 
> $ git diff
> diff --git a/man3type/sockaddr.3type b/man3type/sockaddr.3type
> index 2fdf56c59..e610aa0f5 100644
> --- a/man3type/sockaddr.3type
> +++ b/man3type/sockaddr.3type
> @@ -117,6 +117,14 @@ .SH HISTORY
>  was invented by POSIX.
>  See also
>  .BR accept (2).
> +.PP
> +These structures were invented before modern ISO C strict-aliasing rules.
> +If aliasing rules are applied strictly,
> +these structures would be impossible to use

Maybe "extremely difficult" instead of "impossible" to use (if I
understand this thread correctly, it is possible to memcpy() from one
struct into different storage of a different effective type where the
memcpy()'s intermediate aliasing through char* avoids the UB).

> +without invoking Undefined Behavior (UB).
> +POSIX Issue 8 will fix this by requiring that implementations
> +make sure that these structures
> +can be safely used as they were designed.
>  .SH NOTES
>  .I socklen_t
>  is also defined in
> 
> 
> I guess this is simple enough that it should work as documentation.

It seems fine from my perspective.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[PATCH v3] sockaddr.3type: POSIX Issue 8 will solve strict-aliasing issues with these types

[Alejandro Colomar]

Link: <https://austingroupbugs.net/view.php?id=1641>
Reported-by: Bastien Roucariès <rouca@debian.org>
Reported-by: Alejandro Colomar <alx@kernel.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Cc: glibc <libc-alpha@sourceware.org>
Cc: GCC <gcc@gcc.gnu.org>
Cc: Stefan Puiu <stefan.puiu@gmail.com>
Cc: Igor Sysoev <igor@sysoev.ru>
Cc: Rich Felker <dalias@libc.org>
Cc: Andrew Clayton <andrew@digital-domain.net>
Cc: Richard Biener <richard.guenther@gmail.com>
Cc: Zack Weinberg <zack@owlfolio.org>
Cc: Florian Weimer <fweimer@redhat.com>
Cc: Joseph Myers <joseph@codesourcery.com>
Cc: Jakub Jelinek <jakub@redhat.com>
Cc: Sam James <sam@gentoo.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---

Hi Eric,

I took your informal review as a "Reviewed-by".  Please confirm.
I've also modified the small wording thingy you suggested.

I'll float this patch in the list in case anyone has comments, and
will push some time this weekend (depending on many variables).

Cheers,
Alex

 man3type/sockaddr.3type | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/man3type/sockaddr.3type b/man3type/sockaddr.3type
index 2fdf56c59..cf8d601f5 100644
--- a/man3type/sockaddr.3type
+++ b/man3type/sockaddr.3type
@@ -117,6 +117,14 @@ .SH HISTORY
 was invented by POSIX.
 See also
 .BR accept (2).
+.PP
+These structures were invented before modern ISO C strict-aliasing rules.
+If aliasing rules are applied strictly,
+these structures would be extremely difficult to use
+without invoking Undefined Behavior.
+POSIX Issue 8 will fix this by requiring that implementations
+make sure that these structures
+can be safely used as they were designed.
 .SH NOTES
 .I socklen_t
 is also defined in
-- 
2.40.0

Re: [PATCH v3] sockaddr.3type: POSIX Issue 8 will solve strict-aliasing issues with these types

[Eric Blake]

On Fri, Apr 21, 2023 at 10:27:18PM +0200, Alejandro Colomar wrote:
> Link: <https://austingroupbugs.net/view.php?id=1641>
> Reported-by: Bastien Roucariès <rouca@debian.org>
> Reported-by: Alejandro Colomar <alx@kernel.org>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Cc: glibc <libc-alpha@sourceware.org>
> Cc: GCC <gcc@gcc.gnu.org>
> Cc: Stefan Puiu <stefan.puiu@gmail.com>
> Cc: Igor Sysoev <igor@sysoev.ru>
> Cc: Rich Felker <dalias@libc.org>
> Cc: Andrew Clayton <andrew@digital-domain.net>
> Cc: Richard Biener <richard.guenther@gmail.com>
> Cc: Zack Weinberg <zack@owlfolio.org>
> Cc: Florian Weimer <fweimer@redhat.com>
> Cc: Joseph Myers <joseph@codesourcery.com>
> Cc: Jakub Jelinek <jakub@redhat.com>
> Cc: Sam James <sam@gentoo.org>
> Signed-off-by: Alejandro Colomar <alx@kernel.org>
> ---
> 
> Hi Eric,
> 
> I took your informal review as a "Reviewed-by".  Please confirm.
> I've also modified the small wording thingy you suggested.

For the avoidance of doubt ;)

Reviewed-by: Eric Blake <eblake@redhat.com>


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org