Bug 260117 - [libc][patch] Faulty errno for sctp_getladdrs() in net/sctp_sys_calls.c
Summary: [libc][patch] Faulty errno for sctp_getladdrs() in net/sctp_sys_calls.c
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Michael Tuexen
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2021-11-29 14:01 UTC by Albin
Modified: 2021-12-10 10:48 UTC (History)
2 users (show)

See Also:
tuexen: mfc-stable13+


Attachments
Bug patch (880 bytes, patch)
2021-11-29 14:01 UTC, Albin
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Albin 2021-11-29 14:01:59 UTC
Created attachment 229792 [details]
Bug patch

Regarding the file (net/sctp_sys_calls.c) in libc, I believe that ENOTCONN in sctp_getladdrs() should be placed in sctp_getpaddrs() instead.

Reason is that sctp_getladdrs() is used to get local addresses which doesn't require a connection/association while sctp_getpaddrs() does.

I have provided a patch, but it is untested and I haven't compiled using it.
Comment 1 commit-hook freebsd_committer freebsd_triage 2021-12-01 15:25:54 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=071966e874ed472bdac031b7e89d08bacf8bbbc4

commit 071966e874ed472bdac031b7e89d08bacf8bbbc4
Author:     Michael Tuexen <tuexen@FreeBSD.org>
AuthorDate: 2021-12-01 15:20:17 +0000
Commit:     Michael Tuexen <tuexen@FreeBSD.org>
CommitDate: 2021-12-01 15:25:01 +0000

    libc sctp: fix sctp_getladdrs() when reporting no addresses

    Section 9.5 of RFC 6458 (SCTP Socket API) requires that
    sctp_getladdrs() returns 0 in case the socket is unbound. This
    is the cause of reporting 0 addresses. So don't indicate an
    error, just report this case as required.

    PR:             260117
    MFC after:      1 week

 lib/libc/net/sctp_sys_calls.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
Comment 2 Michael Tuexen freebsd_committer freebsd_triage 2021-12-01 15:28:09 UTC
Thanks for reporting. In the case of an unbound socket, 0 should be returned. This is fixed in https://cgit.FreeBSD.org/src/commit/?id=071966e874ed472bdac031b7e89d08bacf8bbbc4.

Please note that there were a couple of other issues, I fixed separately.
Comment 3 Albin 2021-12-01 15:44:43 UTC
(In reply to Michael Tuexen from comment #2)
Thanks for the quick feedback and help!

Regarding:
https://cgit.freebsd.org/src/commit/?id=071966e874ed472bdac031b7e89d08bacf8bbbc4

Maybe the:
if (size_of_addresses == 0) {

part can stay in the same place as before, then you only need to return 0 and not free addrs.

Also, I checked the RFC and saw this:
"If there is no association on this socket, sctp_getpaddrs() returns 0, and the value of *addrs is undefined."

So maybe a similar if statement can be added to sctp_getpaddrs? Though, I don't know what these existing lines results in if there aren't any association:
if (getsockopt(sd, IPPROTO_SCTP, SCTP_GET_REMOTE_ADDR_SIZE, &size_of_addresses, &opt_len) != 0) {
    return (-1);
}

I am guessing that getsockopt returns 0 even if there aren't any association, so that size_of_addresses could be checked directly afterwards to check if there is an association or not?
Comment 4 commit-hook freebsd_committer freebsd_triage 2021-12-01 19:15:35 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=83a103ec429a6dd862a73857ebeeff863a41a34d

commit 83a103ec429a6dd862a73857ebeeff863a41a34d
Author:     Michael Tuexen <tuexen@FreeBSD.org>
AuthorDate: 2021-12-01 18:47:50 +0000
Commit:     Michael Tuexen <tuexen@FreeBSD.org>
CommitDate: 2021-12-01 18:50:26 +0000

    libc sctp: improve conformance of sctp_getpaddrs()

    When there is no association, don't return -1 and indicate ENOENT,
    but return 0 instead. This is specified in RFC 6458.

    PR:             260117
    MFC after:      1 week

 lib/libc/net/sctp_sys_calls.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
Comment 5 Michael Tuexen freebsd_committer freebsd_triage 2021-12-01 19:19:50 UTC
(In reply to Albin from comment #3)
I moved down the
if (size_of_addresses == 0) {
check, because when using illegal generic association ids, I want to return an error with EINVAL. This is checked by the SCTP_GET_LOCAL_ADDRESSES socket option. The SCTP_GET_LOCAL_ADDR_SIZE socket option does not pay attention to the association id.

I fixed sctp_getpaddrs() in
https://cgit.FreeBSD.org/src/commit/?id=83a103ec429a6dd862a73857ebeeff863a41a34d

Does this address you issues related to sctp_get[lp]addrs()?
Comment 6 Albin 2021-12-02 09:37:38 UTC
(In reply to Michael Tuexen from comment #5)
Oh, it was me who checked the diff wrongly, I thought it still was after SCTP_GET_LOCAL_ADDR_SIZE and didn't see that it was after SCTP_GET_LOCAL_ADDRESSES.

I trust your design choice regarding sctp_getpaddrs.

Yes, thanks for the help and for the improvements!
Comment 7 commit-hook freebsd_committer freebsd_triage 2021-12-10 10:39:19 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=6a5487e34b13f9b7e184a4cca23b715a76f2a375

commit 6a5487e34b13f9b7e184a4cca23b715a76f2a375
Author:     Michael Tuexen <tuexen@FreeBSD.org>
AuthorDate: 2021-12-01 15:20:17 +0000
Commit:     Michael Tuexen <tuexen@FreeBSD.org>
CommitDate: 2021-12-10 10:38:31 +0000

    libc sctp: fix sctp_getladdrs() when reporting no addresses

    Section 9.5 of RFC 6458 (SCTP Socket API) requires that
    sctp_getladdrs() returns 0 in case the socket is unbound. This
    is the cause of reporting 0 addresses. So don't indicate an
    error, just report this case as required.

    PR:             260117
    MFC after:      1 week

    (cherry picked from commit 071966e874ed472bdac031b7e89d08bacf8bbbc4)

 lib/libc/net/sctp_sys_calls.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
Comment 8 commit-hook freebsd_committer freebsd_triage 2021-12-10 10:40:20 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=4bf3c8ea0d107e24c62a96cd5e88c188c629d19b

commit 4bf3c8ea0d107e24c62a96cd5e88c188c629d19b
Author:     Michael Tuexen <tuexen@FreeBSD.org>
AuthorDate: 2021-12-01 18:47:50 +0000
Commit:     Michael Tuexen <tuexen@FreeBSD.org>
CommitDate: 2021-12-10 10:39:15 +0000

    libc sctp: improve conformance of sctp_getpaddrs()

    When there is no association, don't return -1 and indicate ENOENT,
    but return 0 instead. This is specified in RFC 6458.

    PR:             260117
    MFC after:      1 week

    (cherry picked from commit 83a103ec429a6dd862a73857ebeeff863a41a34d)

 lib/libc/net/sctp_sys_calls.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)