tcp, udp: Permit binding with AF_UNSPEC if the address is INADDR_ANY

[markj]

Summary

Prior to commit f161d294b we only checked the sockaddr length, but now
we verify the address family as well. This breaks ttcp. Relax the
check to avoid breaking compatibility too much: permit AF_UNSPEC if the
address is INADDR_ANY. This matches Linux.

Fixes: f161d294b
Reported by: Bakul Shah

[tuexen]

Hmm. What about the corresponding case for IPv6 sockets? ttcp would still fail, since it also provides a sin_len of 0. I was expecting some fallout...

[markj]

In D30539#685837, @tuexen wrote:

Hmm. What about the corresponding case for IPv6 sockets? ttcp would still fail, since it also provides a sin_len of 0. I was expecting some fallout...

Do you know of any applications that depend on that? ttcp is IPv4-only.

sin_len is a funny case. bind(2), connect(2) and sendto(2) also have a socklen parameter. Our getsockaddr(), which is responsible for copying in sockaddrs from user memory, overwrites the sa_len with the length provided in that parameter. So the sin_len provided by userspace is effectively ignored, for better or worse.

[tuexen]

In D30539#685842, @markj wrote:

In D30539#685837, @tuexen wrote:

Hmm. What about the corresponding case for IPv6 sockets? ttcp would still fail, since it also provides a sin_len of 0. I was expecting some fallout...

Do you know of any applications that depend on that? ttcp is IPv4-only.

No, I don't. Just prefer consistency, But if you prefer to add a special case only for IPv4, I'm fine.

sin_len is a funny case. bind(2), connect(2) and sendto(2) also have a socklen parameter. Our getsockaddr(), which is responsible for copying in sockaddrs from user memory, overwrites the sa_len with the length provided in that parameter. So the sin_len provided by userspace is effectively ignored, for better or worse.

Ahh, OK. I missed that. Then I'm fine.

[markj]

In D30539#685845, @tuexen wrote:

In D30539#685842, @markj wrote:

In D30539#685837, @tuexen wrote:

Hmm. What about the corresponding case for IPv6 sockets? ttcp would still fail, since it also provides a sin_len of 0. I was expecting some fallout...

Do you know of any applications that depend on that? ttcp is IPv4-only.

No, I don't. Just prefer consistency, But if you prefer to add a special case only for IPv4, I'm fine.

Well, this way we are more consistent with other kernels. :)

Linux and OpenBSD are strict about checking sa_family == AF_INET6 so I think it is reasonable to do the same.

[zlei]

Hi Mark,

I'd personally like the approach D29519 . Is it desirable to have a dedicated sysctl variable to flag the compatible mode? The default value could be set to true.

[markj]

In D30539#686316, @zlei.huang_gmail.com wrote:

Hi Mark,

I'd personally like the approach D29519 . Is it desirable to have a dedicated sysctl variable to flag the compatible mode? The default value could be set to true.

I'm not sure. This is really a quite minor modification to D29519, I'd personally prefer not to add yet another sysctl for it. Maybe we should make this behaviour conditional on COMPAT_FREEBSD13 instead.

[tuexen]

In D30539#686463, @markj wrote:

In D30539#686316, @zlei.huang_gmail.com wrote:

Hi Mark,

I'd personally like the approach D29519 . Is it desirable to have a dedicated sysctl variable to flag the compatible mode? The default value could be set to true.

I'm not sure. This is really a quite minor modification to D29519, I'd personally prefer not to add yet another sysctl for it. Maybe we should make this behaviour conditional on COMPAT_FREEBSD13 instead.

I agree, no sysctl variable. I'm fine with adding it to COMPAT_FREEBSD13.

[markj]

In D30539#686469, @tuexen wrote:

In D30539#686463, @markj wrote:

In D30539#686316, @zlei.huang_gmail.com wrote:

Hi Mark,

I'd personally like the approach D29519 . Is it desirable to have a dedicated sysctl variable to flag the compatible mode? The default value could be set to true.

I'm not sure. This is really a quite minor modification to D29519, I'd personally prefer not to add yet another sysctl for it. Maybe we should make this behaviour conditional on COMPAT_FREEBSD13 instead.

I agree, no sysctl variable. I'm fine with adding it to COMPAT_FREEBSD13.

I'll do it as a separate change, since this one should be MFCed to stable/13.