Summary: | connect(2): unexpected EADDRINUSE when connecting from IPv6 wildcard to IPv4 address | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | Base System | Reporter: | Dmitri Goutnik <dmgk> | ||||||
Component: | kern | Assignee: | Mike Karels <karels> | ||||||
Status: | Closed FIXED | ||||||||
Severity: | Affects Some People | CC: | emaste, karels, zlei | ||||||
Priority: | --- | Flags: | linimon:
mfc-stable13?
|
||||||
Version: | Unspecified | ||||||||
Hardware: | Any | ||||||||
OS: | Any | ||||||||
Attachments: |
|
Description
Dmitri Goutnik
2022-07-06 15:06:03 UTC
I think it is the same issue as https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=210726 which is marked as fixed but really still here. And there is proposed patch to fix it in that thread (see my last replies). The problem is that the bind() for the failing socket should not have succeeded, and the problem isn't discovered until the connect(). The root problem is that bind on an IPv6 socket is allowed to choose a port that is already bound by a previous socket, but which has been connected and hence turned into an IPv4 socket. The connect cannot succeed then, because it would create a fully duplicate session (4-tuple). I am looking at a fix for the bind, but it is somewhat messy. Oops, added comment for the wrong bug, sorry. (In reply to Mike Karels from comment #3) No, I was looking at the wrong window. The reply was on the correct bug. (In reply to Mike Karels from comment #2) No, wildcard ipv6 bind() shouldn't fail just because ipv4 port for the specifiec ipv4 address is busy. I'm not sure how this error should be reported to userland, but it surely shouldn't be triggered until we try to connect() to ipv6-wrapped ipv4 address. May be it will be okay to return EADDRNOTAVAIL from such connect(), considering wildcard-bound socket as partially unbound socket as we really have to do new ipv4-bind() over existing ipv6-wildcard-bind. (In reply to firk from comment #5) > No, wildcard ipv6 bind() shouldn't fail just because ipv4 port > for the specifiec ipv4 address is busy. There is nothing specific in this bind() call. The address is ANY and the port is zero. Also, the application has cleared the IPV6_ONLY option, specifically enabling IPv4. It seems far better to allocate a port that can work for IPv4 as well as IPv6 in this case. My in-progress change modifies only that situation. Hopefully I'll be ready to circulate the change soon. > I'm not sure how this error should be reported to userland, but it surely > shouldn't be triggered until we try to connect() to ipv6-wrapped ipv4 address. > May be it will be okay to return EADDRNOTAVAIL from such connect(), considering > wildcard-bound socket as partially unbound socket as we really have to do > new ipv4-bind() over existing ipv6-wildcard-bind. We can't do a new bind() for IPv4; ports are immutable once bound. Note that the application could have skipped the bind() call, in which case connect() can do the right thing. But this procedure should also work. Created attachment 235507 [details] patch to fix connect EADDRINUSE (In reply to Mike Karels from comment #6) Ok, I misunderstood the problem before. I think it could be fixed in the same place as already mentioned similar bug 210726. Here is the patch that looks fixing both 210726 and this bug. (In reply to firk from comment #7) Thanks for the new patch. That's simpler (less invasive) than the patch I was working on, and I can simplify it just a little more. One question: why pass NULL for cred? That doesn't seem to affect this case, and seems wrong for other reasons. (In reply to Mike Karels from comment #8) NULL cred is to fix bug 210276. It it not wrong in any case, it means "prevent reusing busy ports regardless of which creds they are using". Obtained struct inpcb never used, we just looking if it exists or not, so no access leak may occur. Made a differential revision for this: https://reviews.freebsd.org/D35967 Created attachment 235512 [details]
alternative patch
I'd prefer to keep the two patches separate, in part because I don't know enough about jails to be sure about that part. This patch also has a simplification, as some of the tests have already been done earlier. Could someone try this with the Go test?
Can someone test my alternative patch (above) with the Go test? btw, this patch passes with the test program. I ran it with a 3-port range, as IPv4 and IPv6 listeners allocate different ports with the patch; two ports leave none for a client. (In reply to Mike Karels from comment #12) I'll test it later today, thanks. Can confirm that the alternative patch fixes the issue. Thanks for looking into this! A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=637f317c6d9c0c689677f499fc78ac545b192071 commit 637f317c6d9c0c689677f499fc78ac545b192071 Author: Mike Karels <karels@FreeBSD.org> AuthorDate: 2022-07-29 14:23:23 +0000 Commit: Mike Karels <karels@FreeBSD.org> CommitDate: 2022-08-02 14:49:46 +0000 IPv6: fix problem with duplicate port assignment with v4-mapped addrs In in_pcb_lport_dest(), if an IPv6 socket does not match any other IPv6 socket using in6_pcblookup_local(), and if the socket can also connect to IPv4 (the INP_IPV4 vflag is set), check for IPv4 matches as well. Otherwise, we can allocate a port that is used by an IPv4 socket (possibly one created from IPv6 via the same procedure), and then connect() can fail with EADDRINUSE, when it could have succeeded if the bound port was not in use. PR: 265064 Submitted by: firk at cantconnect.ru (with modifications) Reviewed by: bz, melifaro Differential Revision: https://reviews.freebsd.org/D36012 sys/netinet/in_pcb.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) Can this be MFH? Thanks. I forgot to add an MFC entry to the commit, but I plan to do it in about a week. A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=1165116ada353364e1d1570d1d23bb3d18d28394 commit 1165116ada353364e1d1570d1d23bb3d18d28394 Author: Mike Karels <karels@FreeBSD.org> AuthorDate: 2022-07-29 14:23:23 +0000 Commit: Ed Maste <emaste@FreeBSD.org> CommitDate: 2024-01-10 13:50:55 +0000 IPv6: fix problem with duplicate port assignment with v4-mapped addrs In in_pcb_lport_dest(), if an IPv6 socket does not match any other IPv6 socket using in6_pcblookup_local(), and if the socket can also connect to IPv4 (the INP_IPV4 vflag is set), check for IPv4 matches as well. Otherwise, we can allocate a port that is used by an IPv4 socket (possibly one created from IPv6 via the same procedure), and then connect() can fail with EADDRINUSE, when it could have succeeded if the bound port was not in use. PR: 265064 Submitted by: firk at cantconnect.ru (with modifications) Reviewed by: bz, melifaro Differential Revision: https://reviews.freebsd.org/D36012 (cherry picked from commit 637f317c6d9c0c689677f499fc78ac545b192071) sys/netinet/in_pcb.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) ^Triage: mfced to 13. |