Two bytes of padding, and a transport that never worked

I was pointing xdp-bfd at FRR’s distributed BFD dataplane socket and wanted the UNIX transport rather than TCP, since both ends were on the same host. It never connected. Every three seconds:

bfdd[13438]: bfd_dplane_client_connect: data plane connection failed: Invalid argument

The TCP transports worked perfectly against the same engine. Only unixc: failed.

The address was fine, the length was not

strace gave it away immediately:

connect(16, {sa_family=AF_UNIX, sun_path="/tmp/frr-repro.sock"}, 112) = -1 EINVAL

The path is right. The 112 is not. On x86_64 with glibc:

sizeof(struct sockaddr_un)  = 110
sizeof(struct sockaddr_in6) = 28
sizeof(union addr)          = 112

bfd_dplane_client_init() copies the caller’s address into a union of sockaddr, sockaddr_in, sockaddr_in6, and sockaddr_un, then records the length. It records the wrong one:

if (salen <= sizeof(bdc->addr)) {
        memcpy(&bdc->addr, sa, salen);
        bdc->addrlen = sizeof(bdc->addr);   /* should be salen */

It stores the size of the whole union instead of the size of the address it was given. The union is 112 bytes because sockaddr_in6 forces 8-byte alignment, padding it past the 110 that sockaddr_un actually occupies.

Why only AF_UNIX noticed

This is the part worth keeping. The kernel’s AF_UNIX path validates the address length strictly. unix_validate_addr() in net/unix/af_unix.c rejects any addrlen greater than sizeof(struct sockaddr_un) outright, so 112 is refused with EINVAL before anything else happens.

Inet sockets do not. Pass an oversized addrlen to an AF_INET connect and it is tolerated, because the kernel only needs the leading bytes it understands. So the exact same bug, on the exact same line, is invisible on the TCP transports and fatal on the UNIX one.

That asymmetry is why this survived so long. git log -S addrlen -- bfdd/dplane.c shows the logic unchanged since client mode was introduced in 6655b43d51. As far as I can tell, unixc: never worked on Linux, in any release.

The fix

memcpy(&bdc->addr, sa, salen);
bdc->addrlen = salen;

One line. The truncating branch keeps sizeof(bdc->addr), which is correct there, because that path really did fill the whole union.

I rebuilt bfdd from the 10.5.1 source with only that change and it connected to the same listener that stock bfdd had been refusing for hours. Merged to master as #22621, reviewed by the dataplane author.

What I took from it

I spent longer than I should have looking at permissions, socket paths, and my own listener, because “Invalid argument” on a connect to a socket that plainly exists reads like the address is wrong. It was not the address. It was the number sitting next to it.

The wider lesson is about the shape of the bug rather than the bug. A single sizeof on a union that holds several address families is a length that happens to be right for the largest member and wrong for the rest. It survived because the family that cares about it strictly was the one nobody was testing.

Packaged releases up to 10.5.1 still carry this, so use the TCP transport with them.