destiny: memcpying with the wrong length

[cgundogan]

when calling posix bind, the user will probably pass a
struct sockaddr_in6 and its corresponding sizeof() return value.
However, in further steps this struct is cast to a sockaddr6_t.
This cast by itself is not dangerous, since the former struct
encapsulates the sockaddr6_t plus an additional field.
But in later steps, memcpy is used with the former sizeof() value,
which was passed to the posix bind call.

Due to different sizes of struct sockaddr_in6 and sockaddr6_t,
the port value of the foreign address field gets overwritten.
But TCP expects the foreign address to be completely zero
in order to handle further connections with this socket.

[Kijewski]

Wouldn't a simple copy be enough?

    get_socket(s)->socket_values.local_address = *name; // ← no memcpy here
    get_socket(s)->socket_values.tcp_control.rto = TCP_INITIAL_ACK_TIMEOUT;
    get_socket(s)->recv_pid = pid;

And caching the result of get_socket() like socket_internal_t *sock = get_socket(s) would be good, too, I guess.

[cgundogan]

I changed the PR to include a simple copy and caching the struct for multiple uses

[Kijewski]

But TCP expects the foreign address to be completely zero
in order to handle further connections with this socket.

Could you please elaborate on this sentence a little bit more. I did not look further into the implementation, and I don't really see the problem. I.e. why do these bits need to be zero?

[cgundogan]

the implementation in destiny iterates through the socket list (in a later step) in order to find an adequate socket for the incoming connection establishment. However, if the foreign_address part of that socket is not NULL, the proper socket is just ignored. (sockets in a listening state have no foreign_address)

The current memcpy uses a copy length, which is bigger than sizeof(copy source). Thus,
the foreign_address, which follows immediatly the local_address within the struct, gets overwritten with corrupt data.

The current implementation was probably correct some time ago, because you would pass directly the sockaddr6_t and a valid length parameter based on that type.

But with the introduction of posix, and the needs to fulfill their API, the user needs to pass a struct sockaddr_in6, which is slightly bigger than the internal sockaddr6_t. But when memcpying, the size of sockaddr_in6 is used.

[mehlis]

@cgundogan do you have any idea what this chunk of code is doing? seems to be some sort of assert check?!

[OlegHahm]

Will be part of #1508