Re: [PATCH] sockaddr.3type: BUGS: Document that libc should be fixed using a union

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

 



Hi Xi,

On 2/6/23 07:02, Xi Ruoyao wrote:
On Sun, 2023-02-05 at 16:31 +0100, Alejandro Colomar via Libc-alpha wrote:

The only correct way to use  different  types  in  an  API  is
through  a  union.

I don't think this statement is true (in general).  Technically we can
write something like this:

struct sockaddr { ... };
struct sockaddr_in { ... };
struct sockaddr_in6 { ... };

int bind(int fd, const struct sockaddr *addr, socklen_t addrlen)
{
     if (addrlen < sizeof(struct sockaddr) {
         errno = EINVAL;
         return -1;
     }

     /* cannot use "addr->sa_family" directly: it will be an UB */
     sa_family_t sa_family;
     memcpy(&sa_family, addr, sizeof(sa_family));

     switch (sa_family) {
         case AF_INET:
             return _do_bind_in(fd, (struct sockaddr_in *)addr, addrlen);
         case AF_INET6:
             return _do_bind_in6(fd, (struct sockaddr_in6 *)addr, addrlen);
         /* more cases follow here */
         default:
             errno = EINVAL;
             return -1;
         }
     }
}

In this way we can use sockaddr_{in,in6,...} for bind() safely, as long
as we can distinguish the "real" type of addr using the leading byte
sequence (and the caller uses it carefully).

True; I hadn't thought of memcpy()ing the first member of the struct. That's valid; overcomplicated but valid.


But obviously sockaddr_storage can't be distinguished here, so casting a
struct sockaddr_stroage * to struct sockaddr * and passing it to bind()
will still be wrong (unless we make sockaddr_storage an union or add
[[gnu::may_alias]]).

But as you say, it still leaves us with a question. What should one declare for passing to the standard APIs? It can only be a union. So we can either tell users to each create their own union, or we can make sockaddr_storage be a union. The latter slightly violates POSIX due to namespaces as Rich noted, but that's a minor violation, and POSIX can be changed to accomodate for that.

If we change sockaddr_storage to be a union, we have two benefits:

- Old code which uses sockaddr_storage is made conforming (non-UB) without modifying the source.
-  Users can inspect the structures.

If we don't, and deprecate sockaddr_storage, we should tell users to declare their own unions _and_ treat all these structures as black boxes which can only be read by memcpy()ing their contents.

Which of the two do we want? I think fixing sockaddr_storage is simpler, and allows existing practice of reading these structures. The other one just makes (or rather acknowledges, since it has always been like that) a lot of existing code invoke UB, and acknowledges that you can't safely use these structures without a lot of workarounding.

Cheers,

Alex

--
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5

Attachment: OpenPGP_signature
Description: OpenPGP digital signature


[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