Austin Group Defect Tracker

Aardvark Mark IV

Main | My View | View Issues | Change Log | Docs
  

Viewing Issue Simple Details [ Jump to Notes ] [ Issue History ] [ Print ]
ID Category Severity Type Date Submitted Last Update
0001641 [1003.1(2016/18)/Issue7+TC2] System Interfaces Editorial Clarification Requested 2023-03-18 07:52 2023-03-22 21:56
Reporter bastien View Status public  
Assigned To
Priority normal Resolution Open  
Status New  
Name Bastien Roucaries
Organization debian
User Reference
Section sys/socket.h
Page Number Application usage
Line Number sockaddr_storage
Interp Status ---
Final Accepted Text
Summary 0001641: sockaddr_storage is not alias safe
Description  sockaddr_storage was designed back when strict aliasing wasn’t a problem.

 Back then, one would define a variable of that type, and then access it as any of the other sockaddr_* types, depending on the value of the first member. This is Undefined Behavior.

However, there is no
way to use these APIs without invoking Undedfined Behavior, either in
the user program or in libc, so it is still recommended to use this
method. The only correct way to use different types in an API is
through a union.

Exemple of safe use
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <sys/un.h>
#include <stdio.h>
#include <unistd.h>
#include <errno.h>
#include <stddef.h>

union sockaddr_mayalias {
  sa_family_t ss_family;
  struct sockaddr sock;
  struct sockaddr_storage storage;
  struct sockaddr_in in;
  struct sockaddr_in6 in6;
  struct sockaddr_un un;
};
  
int main() {
  union sockaddr_mayalias sa = {};
  socklen_t addrlen = sizeof(sa);
  if(getsockname(STDIN_FILENO, &sa.sock, &addrlen) < 0) {
    perror("getsockname");
    return 1;
  }
  if(addrlen >= sizeof(sa)) {
    errno = EPROTONOSUPPORT;
    perror("getsockname return a not supported sock_addr");
    return 1;
  }
  
  switch(sa.ss_family) {
  case(AF_UNSPEC):
    printf("AF_UNSPEC socket\n");
    break;
  case(AF_INET):
    {
      char s[INET_ADDRSTRLEN];
      in_port_t port = ntohs(sa.in.sin_port);
      if (inet_ntop(AF_INET, &(sa.in.sin_addr), s, sizeof(s)) == NULL) {
    perror("inet_ntop");
    return 1;
      }
      printf("AF_INET socket %s:%i\n",s,(int)port);
      break;
    }
  case(AF_INET6):
    {
      char s[INET6_ADDRSTRLEN];
      in_port_t port = ntohs(sa.in6.sin6_port);
      if (inet_ntop(AF_INET6, &(sa.in6.sin6_addr), s, sizeof(s)) == NULL) {
    perror("inet_ntop");
    return 1;
      }
      printf("AF_INET6 socket %s:%i\n",s,(int)port);
      break;
    }
  case(AF_UNIX):
    if(addrlen == sizeof(sa_family_t)) {
      printf("AF_UNIX socket anonymous\n");
      break;
    }
    /* abstract */
    if(sa.un.sun_path[0]=='\0') {
      printf("AF_UNIX abstract socket 0x");
      for (int i = 0; i < (addrlen - sizeof(sa_family_t)); ++i)
    printf("%x",sa.un.sun_path[i]);
      printf("\n");
      break;
    }
    /* named */
    printf("AF_UNIX named socket ");
    for (int i=0; i < strnlen(sa.un.sun_path, addrlen - offsetof(struct sockaddr_un, sun_path));++i)
      printf("%c",sa.un.sun_path[i]);
    printf("\n");
    break;
  default:
      errno = EPROTONOSUPPORT;
      perror("socket not supported");
      return 1;
}

    
}
Desired Action 1. document aliasing problem
2. define sockaddr storage as:
struct sockaddr_storage {
        union {
                sa_family_t ss_family;
                struct sockaddr sa;
                struct sockaddr_in sin;
                struct sockaddr_in6 sin6;
                struct sockaddr_un sun;
                struct _sockaddr_padding padding;
        };
};
Tags No tags attached.
Attached Files

- Relationships

-  Notes
(0006207)
bastien (reporter)
2023-03-18 07:53

 Please note that function that pass sockaddr * instead of void may suffer of the same aliasing problem
(0006208)
wlerch (reporter)
2023-03-20 13:20

 A general note: just because C considers something undefined behaviour does not mean that POSIX can't define it. For example, using dlsym() to return a pointer to a function involves a conversion that C considers undefined, but POSIX requires it to work. It just means that POSIX specifies some additional requirements on compilers that C does not.

Of course, it is probably a good idea to try to keep such additional requirements to a minimum, and mention them explicitly, like the dlsym() page does.
(0006209)
bastien (reporter)
2023-03-20 13:38

 yes I should be documented at least.

Using a __attribute__((__may_alias__)) for the struct attribute will help here, but it is I think simpler to use the plan 2 that is safe and backward compatible.

For the *sockaddr problem for int getpeername, I do not know the safe path.

transparent_union is a solution but it is not always implemented.
(0006211)
steffen (reporter)
2023-03-20 23:06

 The famous Dennis Ritchie 7753@alice.UUCP quote from Article 7844 of comp.lang.c:
 
 The fundamental problem is that it is not possible to write real programs using the X3J11 definition of C. The committee has created an unreal language that no one can or will actually use.

Also the July 15, 2019 LWN.net "Who's afraid of a big bad optimizing compiler?"

Other than that ISO C99 does not know about unnamed unions.
ISO C89 that i comply to as far as possible (a hundred percent with the help of some support macros) also does not.

  (Not to mention that terrible tremendous breath-taking gcc (g++) 3.4.2 bug which did not like unnamed unions and caused real pain on my side.)
(0006212)
hvd (reporter)
2023-03-21 12:01

 Re Note: 0006211: That famous quote is about something that was proposed to be part of the standard but ended up being removed before C90 and is not part of any version of the C standard. The sentence that follows that quote would have made that beyond obvious. I highly suspect you are well aware of this and left out that following sentence deliberately.
(0006215)
sam_james (reporter)
2023-03-21 23:09

 This was discussed extensively recently on the libc-alpha and gcc mailing lists at https://sourceware.org/pipermail/libc-alpha/2023-February/145304.html [^] which was the motivation for this defect report.
(0006216)
steffen (reporter)
2023-03-21 23:33

 I'd very much prefer if there would be an exception like there is one for dlsym, like wlerch has shown in #0006208.

Anything else seems to be a road to madness, as sockaddr_storage is required by the standard (and by real life, mind you) to be correctly aligned for any other socket address type, and then you should be able to cast -- _without_ going over an intermediate (char*), or (char*)(void*), or whatever they want to be done .. to get it done.

Anyway this issue is opened against the wrong standard, because this POSIX bases upon ISO C99, and ISO C99 does not support unnamed unions. Voila.


P.S.: since my C can be compiled with C++ i am still frustrated after more than two decades to have the necessity to use C-style casts to assign to some "virtual table" function pointers, instead of even reinterpret_cast<>.
You know, if i have xy_clone() i want and need to assign it to (*y_clone), and i do so for almost the quarter of a century, and it works. (I assign a function pointer, though mismatch -- the generic vtbl uses void*, say.)
Now compilers complain on the use of "old-style" casts, and i am afraid someday they close the door without replacement. Then i stop flying over that coockoo thing.
Just my one cent.
(0006217)
bastien (reporter)
2023-03-22 09:42

 @steffen do not supporting aliasing in general will be no way from compiler folk...

And it is not the wrong standard, the problem is for historical reason socket API pass struct sockaddr* and not void *.

Using void * will have avoided this issue.

the __attribute__((__may_alias__)) is a common C extension and will solve this problem.

Transparent union is also a common supported extension.

May be a safe path will be:
1. Prefered option struct sockaddr and struct sockaddr_storage may alias other structure
2. If compiler does not support this common extension, aliasing should be disabled
(0006227)
steffen (reporter)
2023-03-22 21:56

 No. "Issue 8 drafts" is maybe the right standard.

I say maybe. I personally am the wrong person to talk to, i hate this aliasing issue, i saw so-and-so many *BSD and more commits fly by where code that worked started breaking, and had to be circumvented by doing memcpy(), sheer grazy.

They _broke_ the system. And i am yet to see the advantages. (From my simple userspace programmer's point of view, with a bit of assembler, in earlier times.)
So if an intermediate cast to "char*" gets you going, or "void*", why do that at all?
It _will_ fit in the size of sockaddr_storage, it will have the proper alignment, i want to cast it. Period.

"POSIX-compliant compilers" (-std=c99) should not fail in theory.
Issue 7 is based upon ISO C99.

- Issue History
Date Modified Username Field Change
2023-03-18 07:52 bastien New Issue
2023-03-18 07:52 bastien Name => Bastien Roucaries
2023-03-18 07:52 bastien Organization => debian
2023-03-18 07:52 bastien Section => sys/socket.h
2023-03-18 07:52 bastien Page Number => Application usage
2023-03-18 07:52 bastien Line Number => sockaddr_storage
2023-03-18 07:53 bastien Note Added: 0006207
2023-03-18 07:53 bastien Issue Monitored: bastien
2023-03-20 13:20 wlerch Note Added: 0006208
2023-03-20 13:38 bastien Note Added: 0006209
2023-03-20 23:06 steffen Note Added: 0006211
2023-03-21 12:01 hvd Note Added: 0006212
2023-03-21 23:09 sam_james Note Added: 0006215
2023-03-21 23:33 steffen Note Added: 0006216
2023-03-22 09:42 bastien Note Added: 0006217
2023-03-22 21:56 steffen Note Added: 0006227

Mantis 1.1.6[^]
Copyright © 2000 - 2008 Mantis Group
Powered by Mantis Bugtracker