Le jeudi 19 janvier 2023, 12:42:52 UTC Alejandro Colomar a écrit : Hi all, [adding Eric Blake from redhat and if I remember well member of POSIX comitee] Eric to be short posix behavior of sockaddr sockaddr_storage is UB and example supplied are UB. See here: https://pubs.opengroup.org/onlinepubs/009696699/basedefs/sys/socket.h.html I think we could save the day by defining struct sockadd_storage using anonymous union (thanks to C11 support), but it will leave C++. But how can be done... > Hi Bastien, > Hi Bastien, > > On 1/17/23 11:22, Bastien Roucariès wrote: > > Hi, > > > > I have a lot of student that does not use correctly getsockname in case of > > dual stack. > > Please show some examples of common mistakes, if you can. oh the basic one is to pass a sockaddr_in instead of sockaddr_in6... Or to pass a sockaddr_in6 and do not think that it could be returned an IPV4 socket or an Unix stream socket. > > > > > May be this kind of discussion should be factorized in sockaddr_storage (the > > historical note particularly). > > > > i suppose the same should be done for getpeername > > > > I think a safe programming example may be given that accept a socket as stdin > > and print information on it. Using socat it could be simple to test. > > Please provide some if you can. However, be careful, since it might easily fall > into Undefined Behavior. Ok see it attached. > > > maybe > > forcing ENOTSUPP if *addr > sizeof(sockadd_storage) > > > > Regards > > > > Bastien > > I'll add some comments to the patch. > > > From 0afb3ad23f8ea09331f21a377e3ad19c44e4df18 Mon Sep 17 00:00:00 2001 > > From: =?UTF-8?q?Bastien=20Roucari=C3=A8s?= <rouca@xxxxxxxxxx> > > Date: Tue, 17 Jan 2023 10:07:43 +0000 > > Subject: [PATCH] Document use of struct sockaddr_storage in getsockname > > > > Document: > > - storage requierement > > - future compatibility > > --- > > man2/getsockname.2 | 56 ++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 56 insertions(+) > > > > diff --git a/man2/getsockname.2 b/man2/getsockname.2 > > index e6e8980c9..5914c9e12 100644 > > --- a/man2/getsockname.2 > > +++ b/man2/getsockname.2 > > @@ -39,6 +39,17 @@ The returned address is truncated if the buffer provided is too small; > > in this case, > > .I addrlen > > will return a value greater than was supplied to the call. > > +.PP > > +For greater portability > > +.I addr > > +should point to a structure of type > > +.I sockaddr_storage. > > This is not correct. If the object has a type of 'struct sockaddr_storage', > then it can only be accessed as a 'struct sockaddr_storage'. Trying to access > it as any other structure type would be Undefined Behavior. The way to do it > conforming to ISO C would be to declare a union which contains all the 'struct > sockaddr_*' that are interesting, and access it through the correct union > member. Anything else is on UB land. The only use of 'struct sockaddr_storage' > that I can think of, is for having a more consistent union size. Ok I see, Eric could we redefine at posix level the struct sockaddr_storage to be something like, it seems that it is allowed by /* * Following fields are implementation-defined. */ union sockaddr_storage { sa_family_t ss_family; struct sockaddr sock; struct _sockaddr_storage storage; struct sockaddr_in in; struct sockaddr_in6 in6; struct sockaddr_un un; }; > > > +.I sockaddr_storage > > +structure is large enough to hold any of the other > > +.I sockaddr_* > > +variants and always well aligned. On return, it should be cast to the correct > > +.I sockaddr_* > > The fact that it is correctly aligned, and a cast will work most of the time, > isn't enough for strict aliasing rules. The compiler is free to assume things, > just by the fact that it's a different type. Ok any idea for writing this kind of stuff > > Cheers, > > Alex > > > +type, according to the current protocol family, given by the member ss_family. > > .SH RETURN VALUE > > On success, zero is returned. > > On error, \-1 is returned, and > > @@ -80,10 +91,55 @@ For background on the > > .I socklen_t > > type, see > > .BR accept (2). > > +.PP > > +Security and portability wise, use of > > +.I struct sockaddr_storage > > +type as > > +.I addr > > +and > > +.I addrlen > > +set to > > +.BI "sizeof(struct sockaddr_storage)" > > +is strongly encouraged. Particularly this usage avoid bugs in dual stack IPv4+IPv6 configuration. > > +.PP > > +Historical use of > > +.I addr > > +requires one to use a structure specific to the protocol family in use, > > +such as > > +.I sockaddr_in > > +(AF_INET or IPv4), > > +.I sockaddr_in6 > > +(AF_INET6 or IPv6), or > > +.I sockaddr_un > > +(AF_UNIX) > > +cast to a > > +.I (struct sockaddr *). > > +The purpose of the > > +.I struct sockaddr * > > +type > > +is purely to allow casting of domain-specific socket address types to a > > +"generic" type, so as to avoid compiler warnings about type mismatches in calls to the sockets API. > > +Nevertheless, > > +. I struct sockaddr * > > +is too small to hold newer protocol address (for instance IPv6) and not always well aligned. > > +.PP > > +Even if > > +.I sockaddr_storage > > +type is large enough at compilation time to hold any of the > > +.I sockaddr_* > > +variants known by the library, > > +this guarantee will not hold for future. Newer protocol may need an increase of > > +.I sockaddr_storage > > +size or alignement requirement, and safe program should always runtime check if returned > > +.I addr > > +is smaller or equal to compile time > > +.BI "sizeof(struct sockaddr_storage)" > > +size. > > .SH SEE ALSO > > .BR bind (2), > > .BR socket (2), > > .BR getifaddrs (3), > > +.BR sockaddr_storage (3type), > > .BR ip (7), > > .BR socket (7), > > .BR unix (7) > > -- > > 2.39.0 > > > > -- > <http://www.alejandro-colomar.es/> >
#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;
}
}
Attachment:
signature.asc
Description: This is a digitally signed message part.
