netinet: Fix sockaddr type confusion in_pcb_lport_dest()

[markj]

Summary

in_pcb_lport_dest() uses the inp vflag to determine whether the caller
passed in a v4 or a v6 address. This breaks in the face of v4-mapped
IPv6 addresses, wherein the caller will pass a v4 address. In
particular, the test
(inp->inp_vflag & (INP_IPV4 | INP_IPV6)) == INP_IPV4 fails in this case,
and we end up treating a sockaddr_in as a sockaddr_in6.

Instead, use the sockaddr family to determine what to do, assuming that
the caller has done any required checking (e.g., checking that
IN6P_IPV6_V6ONLY is unset). We can assume that the local and foreign
addresses are from the same family.

Reported by: [e-mail]
Reported by: [e-mail]

[• karels]

I think this looks right as far as it goes.

sys/netinet/in_pcb.c

811

Do this test and the one at line 816 look correct still? Or should they test both vflag and the address family? I think the worst case is if laddr is used when it is INADDR_ANY but shouldn't be used.

[bz]

sys/netinet/in_pcb.c

711

Can you print fsa and lsa pointers and sa_families (if xsa is not NULL) in the message so they'll be on the console; so much easier to debug with users then.

[markj]

sys/netinet/in_pcb.c

811

Hmm, this is a bit tricky since we might not have an address family to check here, and the PCB doesn't tell us the address format it's using for its laddr and faddr.

I note that when binding or connecting to a v4-mapped v6 address, protocol layers (e.g., udp6_bind()) will clear INP_IPV6 before updating the inpcb. TCP will also do this for an implicit connect(), but UDP does not, which seems like a bug. Maybe that's the real bug here - I think this patch is not needed if protocols ensure that only one of INP_IPV4/V6 is set before calling this function.

[markj]

sys/netinet/in_pcb.c

811

Hmm, that might be intentional after all since a UDP socket can be connected and disconnected multiple times in its life cycle. But I don't understand how that works in the face of in_pcbinshash() and in_pcbrehash(), which use the vflag to select whether to use a v4 or a v6 hash table. For v4-mapped addresses, don't we want to use the v4 hash table?

It seems like udp_send() should indeed clear INP_IPV6 when connecting to a v4-mapped address, and udp6_disconnect() should re-set INP_IPV6.

[bz]

sys/netinet/in_pcb.c

811

I think it might be worth going back to FreeBSD 11/10 or so and see there how it once worked and where we changed from v6->v4 (sock)addr there. I have a suspicious feeling that this was half-done with a lot of updates but never looked at the full picture in a while.

[markj]

Fixed in a different way in https://cgit.FreeBSD.org/src/commit/?id=abca3ae7734f664ee9c5edc7a9d3a17e29180bdb