Page MenuHomeFreeBSD

netinet: Fix sockaddr type confusion in_pcb_lport_dest()
Needs ReviewPublic

Authored by markj on Mar 22 2023, 6:03 PM.
Referenced Files
Unknown Object (File)
Mon, May 15, 5:38 AM
Unknown Object (File)
Sun, May 14, 9:14 AM
Unknown Object (File)
Wed, May 3, 10:28 PM
Unknown Object (File)
Apr 26 2023, 3:32 PM
Unknown Object (File)
Mar 30 2023, 4:49 AM


Group Reviewers

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:
Reported by:

Diff Detail

rG FreeBSD src repository
Lint Skipped
Tests Skipped
Build Status
Buildable 50528
Build 47419: arc lint + arc unit

Event Timeline

markj requested review of this revision.Mar 22 2023, 6:03 PM

I think this looks right as far as it goes.


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 added inline comments.

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 added inline comments.

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.


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.


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.