Re: [1003.1(2016/18)/Issue7+TC2 0001641]: sockaddr_storage is not alias safe

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

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

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.

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




[Index of Archives]     [Kernel Documentation]     [Netdev]     [Linux Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux