Bug 265064 - connect(2): unexpected EADDRINUSE when connecting from IPv6 wildcard to IPv4 address
Summary: connect(2): unexpected EADDRINUSE when connecting from IPv6 wildcard to IPv4 ...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: Unspecified
Hardware: Any Any
: --- Affects Some People
Assignee: Mike Karels
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-07-06 15:06 UTC by Dmitri Goutnik
Modified: 2024-01-20 18:09 UTC (History)
3 users (show)

See Also:
linimon: mfc-stable13?


Attachments
patch to fix connect EADDRINUSE (1.11 KB, patch)
2022-07-27 13:52 UTC, firk
no flags Details | Diff
alternative patch (1.01 KB, patch)
2022-07-27 20:05 UTC, Mike Karels
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitri Goutnik freebsd_committer freebsd_triage 2022-07-06 15:06:03 UTC
I'm trying to get to the bottom of the Go issue reported in [1] and noticed some differences in the bind(2)/connect(2) behavior between IPv4/IPv6 and IPv4-mapped addresses. I wrote a small C reproducer [2] to isolate the issue.

When the available port range is exhausted, both IPv4->IPv4 and IPv6->IPv6 clients fail to bind(2) with EADDRNOTAVAIL, which is expected:

$ sysctl net.inet.ip.portrange.first=10000
$ sysctl net.inet.ip.portrange.last=10001
$ ./localdial -4
server4: listening on 0.0.0.0:10001
server6: listening on :::10001
client4: connecting          0.0.0.0:0     -> 127.0.0.1:10001
client4: connected         127.0.0.1:10000 -> 127.0.0.1:10001
server4: accepted          127.0.0.1:10000 ->   0.0.0.0:10001
client4: connecting          0.0.0.0:0     -> 127.0.0.1:10001
localdial: client4: bind: Can't assign requested address
$ ./localdial -6
server4: listening on 0.0.0.0:10001
server6: listening on :::10001
client6: connecting               :::0     ->       ::1:10001
client6: connected               ::1:10000 ->       ::1:10001
client6: connecting               :::0     ->       ::1:10001
server6: accepted                ::1:10000 ->        :::10001
localdial: client6: bind: Can't assign requested address

With IPv4-mapped address, the behavior is different:

$ sysctl net.inet.ip.portrange.first=10000
$ sysctl net.inet.ip.portrange.last=10001
$ ./localdial -M
server4: listening on 0.0.0.0:10001
server6: listening on :::10001
client6: connecting               :::0     -> ::ffff:127.0.0.1:10001
client6: connected  ::ffff:127.0.0.1:10000 -> ::ffff:127.0.0.1:10001
server4: accepted          127.0.0.1:10000 ->   0.0.0.0:10001
client6: connecting               :::0     -> ::ffff:127.0.0.1:10001
client6: connected  ::ffff:127.0.0.1:10000 -> ::ffff:127.0.0.1:10001
server4: accepted          127.0.0.1:10000 ->   0.0.0.0:10001
client6: connecting               :::0     -> ::ffff:127.0.0.1:10001
localdial: client6: connect: Address already in use

Here, the client successfully does bind(2) but then fails at connect(2) with EADDRINUSE, which looks surprising. This seems to confuse the Go testing suite and leads to [1].

Is this an expected/known issue? Reproducible on all FreeBSD versions from 12.3-RELEASE to 14.0-CURRENT.

[1] https://github.com/golang/go/issues/34264
[2] https://github.com/dmgk/localdial
Comment 1 firk 2022-07-23 17:07:22 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).
Comment 2 Mike Karels freebsd_committer freebsd_triage 2022-07-24 22:12:12 UTC
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.
Comment 3 Mike Karels freebsd_committer freebsd_triage 2022-07-24 22:12:54 UTC
Oops, added comment for the wrong bug, sorry.
Comment 4 Mike Karels freebsd_committer freebsd_triage 2022-07-24 22:15:40 UTC
(In reply to Mike Karels from comment #3)

No, I was looking at the wrong window.  The reply was on the correct bug.
Comment 5 firk 2022-07-26 14:56:52 UTC
(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.
Comment 6 Mike Karels freebsd_committer freebsd_triage 2022-07-26 21:35:05 UTC
(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.
Comment 7 firk 2022-07-27 13:52:02 UTC
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.
Comment 8 Mike Karels freebsd_committer freebsd_triage 2022-07-27 16:25:14 UTC
(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.
Comment 9 firk 2022-07-27 17:45:26 UTC
(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.
Comment 10 firk 2022-07-27 19:35:12 UTC
Made a differential revision for this: https://reviews.freebsd.org/D35967
Comment 11 Mike Karels freebsd_committer freebsd_triage 2022-07-27 20:05:13 UTC
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?
Comment 12 Mike Karels freebsd_committer freebsd_triage 2022-08-01 15:19:36 UTC
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.
Comment 13 Dmitri Goutnik freebsd_committer freebsd_triage 2022-08-01 15:26:53 UTC
(In reply to Mike Karels from comment #12)
I'll test it later today, thanks.
Comment 14 Dmitri Goutnik freebsd_committer freebsd_triage 2022-08-01 18:49:02 UTC
Can confirm that the alternative patch fixes the issue. Thanks for looking into this!
Comment 15 commit-hook freebsd_committer freebsd_triage 2022-08-02 14:51:33 UTC
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(-)
Comment 16 Dmitri Goutnik freebsd_committer freebsd_triage 2022-08-02 20:35:24 UTC
Can this be MFH? Thanks.
Comment 17 Mike Karels freebsd_committer freebsd_triage 2022-08-02 21:10:55 UTC
I forgot to add an MFC entry to the commit, but I plan to do it in about a week.
Comment 18 commit-hook freebsd_committer freebsd_triage 2024-01-10 14:20:57 UTC
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(-)
Comment 19 Mark Linimon freebsd_committer freebsd_triage 2024-01-20 18:09:25 UTC
^Triage: mfced to 13.