Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

will casting around sockaddr_storage and sockaddr_in break strict aliasing

Following my previous question, I'm really curious about this code -

case AF_INET: 
    {
        struct sockaddr_in * tmp =
            reinterpret_cast<struct sockaddr_in *> (&addrStruct);
        tmp->sin_family = AF_INET;
        tmp->sin_port = htons(port);
        inet_pton(AF_INET, addr, tmp->sin_addr);
    }
    break;

Before asking this question, I've searched across SO about same topic and have got mix responses about this topic. For example, see this, this and this post which say that it is somehow safe to use this kind of code. Also there's another post that says to use unions for such task but again the comments on accepted answer beg to differ.


Microsoft's documentation on same structure says -

Application developers normally use only the ss_family member of the SOCKADDR_STORAGE. The remaining members ensure that the SOCKADDR_STORAGE can contain either an IPv6 or IPv4 address and the structure is padded appropriately to achieve 64-bit alignment. Such alignment enables protocol-specific socket address data structures to access fields within a SOCKADDR_STORAGE structure without alignment problems. With its padding, the SOCKADDR_STORAGE structure is 128 bytes in length.

Opengroup's documentation states -

The header shall define the sockaddr_storage structure. This structure shall be:

Large enough to accommodate all supported protocol-specific address structures

Aligned at an appropriate boundary so that pointers to it can be cast as pointers to protocol-specific address structures and used to access the fields of those structures without alignment problems

Man page of socket also says same -

In addition, the sockets API provides the data type struct sockaddr_storage. This type is suitable to accommodate all supported domain-specific socket address structures; it is large enough and is aligned properly. (In particular, it is large enough to hold IPv6 socket addresses.)


I've seen multiple implementation using such casts in both C and C++ languages in the wild and now I'm uncertain of the fact which one is right since there are some posts that contradict with above claims - this and this.

So which one is the safe and right way to fill up a sockaddr_storage structure? Are these pointer casts safe? or the union method? I'm also aware of the getaddrinfo() call but that seems a little complicated for the above task of just filling the structs. There is one other recommended way with memcpy, is this safe?

like image 595
Abhinav Gauniyal Avatar asked Feb 11 '17 16:02

Abhinav Gauniyal


2 Answers

C and C++ compilers have become much more sophisticated in the past decade than they were when the sockaddr interfaces were designed, or even when C99 was written. As part of that, the understood purpose of "undefined behavior" has changed. Back in the day, undefined behavior was usually intended to cover disagreement among hardware implementations as to what the semantics of an operation was. But nowadays, thanks ultimately to a number of organizations who wanted to stop having to write FORTRAN and could afford to pay compiler engineers to make that happen, undefined behavior is a thing that compilers use to make inferences about the code. Left shift is a good example: C99 6.5.7p3,4 (rearranged a little for clarity) reads

The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated bits are filled with zeros. If the value of [E2] is negative or is greater than or equal to the width of the promoted [E1], the behavior is undefined.

So, for instance, 1u << 33 is UB on a platform where unsigned int is 32 bits wide. The committee made this undefined because different CPU architectures' left-shift instructions do different things in this case: some produce zero consistently, some reduce the shift count modulo the width of the type (x86), some reduce the shift count modulo some larger number (ARM), and at least one historically-common architecture would trap (I don't know which one, but that's why it's undefined and not unspecified). But nowadays, if you write

unsigned int left_shift(unsigned int x, unsigned int y)
{ return x << y; }

on a platform with 32-bit unsigned int, the compiler, knowing the above UB rule, will infer that y must have a value in the range 0 through 31 when the function is called. It will feed that range into interprocedural analysis, and use it to do things like remove unnecessary range checks in the callers. If the programmer has reason to think they aren't unnecessary, well, now you begin to see why this topic is such a can of worms. (Modern compilers can optimize x << (y&31) into a single shift instruction for ISAs like x86 where the shift instruction implements that masking.)

For more on this change in the purpose of undefined behavior, please see the LLVM people's three-part essay on the subject (1 2 3).


Now that you understand that, I can actually answer your question.

These are the definitions of struct sockaddr, struct sockaddr_in, and struct sockaddr_storage, after eliding some irrelevant complications:

struct sockaddr {
    uint16_t sa_family;
};
struct sockaddr_in { 
    uint16_t sin_family;
    uint16_t sin_port;
    uint32_t sin_addr;
};
struct sockaddr_storage {
    uint16_t ss_family;
    char __ss_storage[128 - (sizeof(uint16_t) + sizeof(unsigned long))];
    unsigned long int __ss_force_alignment;
};

This is poor man's subclassing. It is a ubiquitous idiom in C. You define a set of structures that all have the same initial field, which is a code number that tells you which structure you've actually been passed. Back in the day, everyone expected that if you allocated and filled in a struct sockaddr_in, upcast it to struct sockaddr, and passed it to e.g. connect, the implementation of connect could dereference the struct sockaddr pointer safely to retrieve the sa_family field, learn that it was looking at a sockaddr_in, cast it back, and proceed. The C standard has always said that dereferencing the struct sockaddr pointer triggers undefined behavior—those rules are unchanged since C89—but everyone expected that it would be safe in this case because it would be the same "load 16 bits" instruction no matter which structure you were really working with. That's why POSIX and the Windows documentation talk about alignment; the people who wrote those specs, back in the 1990s, thought that the primary way this could actually be trouble was if you wound up issuing a misaligned memory access.

But the text of the standard doesn't say anything about load instructions, nor alignment. This is what it says (C99 §6.5p7 + footnote):

An object shall have its stored value accessed only by an lvalue expression that has one of the following types:73)

  • a type compatible with the effective type of the object,
  • a qualified version of a type compatible with the effective type of the object,
  • a type that is the signed or unsigned type corresponding to the effective type of the object,
  • a type that is the signed or unsigned type corresponding to a qualified version of the effective type of the object,
  • an aggregate or union type that includes one of the aforementioned types among its members (including, recursively, a member of a subaggregate or contained union), or
  • a character type.

73) The intent of this list is to specify those circumstances in which an object may or may not be aliased.

struct types are "compatible" only with themselves, and the "effective type" of a declared variable is its declared type. So the code you showed...

struct sockaddr_storage addrStruct;
/* ... */
case AF_INET: 
{
    struct sockaddr_in * tmp = (struct sockaddr_in *)&addrStruct;
    tmp->sin_family = AF_INET;
    tmp->sin_port = htons(port);
    inet_pton(AF_INET, addr, tmp->sin_addr);
}
break;

... has undefined behavior, and compilers can make inferences from that, even though naive code generation would behave as expected. What a modern compiler is likely to infer from this is that the case AF_INET can never be executed. It will delete the entire block as dead code, and hilarity will ensue.


So how do you work with sockaddr safely? The shortest answer is "just use getaddrinfo and getnameinfo." They deal with this problem for you.

But maybe you need to work with an address family, such as AF_UNIX, that getaddrinfo doesn't handle. In most cases you can just declare a variable of the correct type for the address family, and cast it only when calling functions that take a struct sockaddr *

int connect_to_unix_socket(const char *path, int type)
{
    struct sockaddr_un sun;
    size_t plen = strlen(path);
    if (plen >= sizeof(sun.sun_path)) {
        errno = ENAMETOOLONG;
        return -1;
    }
    sun.sun_family = AF_UNIX;
    memcpy(sun.sun_path, path, plen+1);

    int sock = socket(AF_UNIX, type, 0);
    if (sock == -1) return -1;

    if (connect(sock, (struct sockaddr *)&sun,
                offsetof(struct sockaddr_un, sun_path) + plen)) {
        int save_errno = errno;
        close(sock);
        errno = save_errno;
        return -1;
    }
    return sock;
}

The implementation of connect has to jump through some hoops to make this safe, but that is Not Your Problem.

Contra the other answer, there is one case where you might want to use sockaddr_storage; in conjunction with getpeername and getnameinfo, in a server that needs to handle both IPv4 and IPv6 addresses. It is a convenient way to know how big of a buffer to allocate.

#ifndef NI_IDN
#define NI_IDN 0
#endif
char *get_peer_hostname(int sock)
{
    char addrbuf[sizeof(struct sockaddr_storage)];
    socklen_t addrlen = sizeof addrbuf;

    if (getpeername(sock, (struct sockaddr *)addrbuf, &addrlen))
        return 0;

    char *peer_hostname = malloc(MAX_HOSTNAME_LEN+1);
    if (!peer_hostname) return 0;

    if (getnameinfo((struct sockaddr *)addrbuf, addrlen,
                    peer_hostname, MAX_HOSTNAME_LEN+1,
                    0, 0, NI_IDN) {
        free(peer_hostname);
        return 0;
    }
    return peer_hostname;
}

(I could just as well have written struct sockaddr_storage addrbuf, but I wanted to emphasize that I never actually need to access the contents of addrbuf directly.)

A final note: if the BSD folks had defined the sockaddr structures just a little bit differently ...

struct sockaddr {
    uint16_t sa_family;
};
struct sockaddr_in { 
    struct sockaddr sin_base;
    uint16_t sin_port;
    uint32_t sin_addr;
};
struct sockaddr_storage {
    struct sockaddr ss_base;
    char __ss_storage[128 - (sizeof(uint16_t) + sizeof(unsigned long))];
    unsigned long int __ss_force_alignment;
};

... upcasts and downcasts would have been perfectly well-defined, thanks to the "aggregate or union that includes one of the aforementioned types" rule. If you're wondering how you should deal with this problem in new C code, here you go.

like image 84
zwol Avatar answered Nov 02 '22 11:11

zwol


Yes, it's an aliasing violation to do this. So don't. There's no need to ever use sockaddr_storage; it was a historical mistake. But there are a few safe ways to use it:

  1. malloc(sizeof(struct sockaddr_storage)). In this case, the pointed-to memory does not have an effective type until you store something to it.
  2. As part of a union, explicitly accessing the member you want. But in this case just put the actual sockaddr types you want (in and in6 and maybe un) in the union rather than sockaddr_storage.

Of course in modern programming you should never need to create objects of type struct sockaddr_* at all. Simply use getaddrinfo and getnameinfo to translate addresses between string representations and sockaddr objects, and treat the latter as completely opaque objects.

like image 6
R.. GitHub STOP HELPING ICE Avatar answered Nov 02 '22 12:11

R.. GitHub STOP HELPING ICE