Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use SOCK_CLOEXEC and accept4() on more platforms. #78572

Merged
merged 2 commits into from
Nov 8, 2020

Conversation

de-vri-es
Copy link
Contributor

@de-vri-es de-vri-es commented Oct 30, 2020

This PR enables the use of SOCK_CLOEXEC and accept4 on more platforms.


Android uses the linux kernel, so it should also support it.

DragonflyBSD introduced them in 4.4 (December 2015):
https://www.dragonflybsd.org/release44/

FreeBSD introduced them in 10.0 (January 2014):
https://wiki.freebsd.org/AtomicCloseOnExec

Illumos introduced them in a commit in April 2013, not sure when it was released. It is quite possible that is has always been in Illumos:
illumos/illumos-gate@5dbfd19
https://illumos.org/man/3socket/socket
https://illumos.org/man/3socket/accept4

NetBSD introduced them in 6.0 (Oktober 2012) and 8.0 (July 2018):
https://man.netbsd.org/NetBSD-6.0/socket.2
https://man.netbsd.org/NetBSD-8.0/accept.2

OpenBSD introduced them in 5.7 (May 2015):
https://man.openbsd.org/socket https://man.openbsd.org/accept

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @KodrAus (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 30, 2020
@m-ou-se m-ou-se added O-android Operating system: Android O-dragonfly Operating system: DragonFly BSD O-freebsd Operating system: FreeBSD O-netbsd Operating system: NetBSD O-openbsd Operating system: OpenBSD T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Oct 30, 2020
@m-ou-se
Copy link
Member

m-ou-se commented Oct 30, 2020

NetBSD introduced them in 6.0 (Oktober 2012) and 8.0 (July 2018):

Looks like the CI uses a NetBSD from March 2018:

curl $URL/2018-03-01-netbsd-src.tgz | tar xzf -

Edit: But libc does have it for netbsd. So should pass the CI fine.

@de-vri-es
Copy link
Contributor Author

de-vri-es commented Oct 30, 2020

Also note that NetBSD 7 is no longer supported since June 30 2020: https://www.netbsd.org/releases/formal.html#previous

So assuming 8.0 or later should be fine I think. libc came to the same conclusion :)

@m-ou-se
Copy link
Member

m-ou-se commented Oct 30, 2020

Alright, sounds good. Let's see if it passes the CI. :)

@bors r+

@bors
Copy link
Contributor

bors commented Oct 30, 2020

📌 Commit 59c6ae6 has been approved by m-ou-se

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 30, 2020
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Oct 30, 2020
Use SOCK_CLOEXEC and accept4() on more platforms.

This PR enables the use of `SOCK_CLOEXEC` and `accept4` on more platforms.

-----

Android uses the linux kernel, so it should also support it.

DragonflyBSD introduced them in 4.4 (December 2015):
https://www.dragonflybsd.org/release44/

FreeBSD introduced them in 10.0 (January 2014):
https://wiki.freebsd.org/AtomicCloseOnExec

Illumos introduced them in a commit in April 2013, not sure when it was released. It is quite possible that is has always been in Illumos:
illumos/illumos-gate@5dbfd19
https://illumos.org/man/3socket/socket
https://illumos.org/man/3socket/accept4

NetBSD introduced them in 6.0 (Oktober 2012) and 8.0 (July 2018):
https://man.netbsd.org/NetBSD-6.0/socket.2
https://man.netbsd.org/NetBSD-8.0/accept.2

OpenBSD introduced them in 5.7 (May 2015):
https://man.openbsd.org/socket https://man.openbsd.org/accept
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Oct 31, 2020
Use SOCK_CLOEXEC and accept4() on more platforms.

This PR enables the use of `SOCK_CLOEXEC` and `accept4` on more platforms.

-----

Android uses the linux kernel, so it should also support it.

DragonflyBSD introduced them in 4.4 (December 2015):
https://www.dragonflybsd.org/release44/

FreeBSD introduced them in 10.0 (January 2014):
https://wiki.freebsd.org/AtomicCloseOnExec

Illumos introduced them in a commit in April 2013, not sure when it was released. It is quite possible that is has always been in Illumos:
illumos/illumos-gate@5dbfd19
https://illumos.org/man/3socket/socket
https://illumos.org/man/3socket/accept4

NetBSD introduced them in 6.0 (Oktober 2012) and 8.0 (July 2018):
https://man.netbsd.org/NetBSD-6.0/socket.2
https://man.netbsd.org/NetBSD-8.0/accept.2

OpenBSD introduced them in 5.7 (May 2015):
https://man.openbsd.org/socket https://man.openbsd.org/accept
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Oct 31, 2020
Use SOCK_CLOEXEC and accept4() on more platforms.

This PR enables the use of `SOCK_CLOEXEC` and `accept4` on more platforms.

-----

Android uses the linux kernel, so it should also support it.

DragonflyBSD introduced them in 4.4 (December 2015):
https://www.dragonflybsd.org/release44/

FreeBSD introduced them in 10.0 (January 2014):
https://wiki.freebsd.org/AtomicCloseOnExec

Illumos introduced them in a commit in April 2013, not sure when it was released. It is quite possible that is has always been in Illumos:
illumos/illumos-gate@5dbfd19
https://illumos.org/man/3socket/socket
https://illumos.org/man/3socket/accept4

NetBSD introduced them in 6.0 (Oktober 2012) and 8.0 (July 2018):
https://man.netbsd.org/NetBSD-6.0/socket.2
https://man.netbsd.org/NetBSD-8.0/accept.2

OpenBSD introduced them in 5.7 (May 2015):
https://man.openbsd.org/socket https://man.openbsd.org/accept
@m-ou-se
Copy link
Member

m-ou-se commented Oct 31, 2020

This failed in #78589 (comment)

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 31, 2020
@de-vri-es
Copy link
Contributor Author

de-vri-es commented Oct 31, 2020

Interesting. The libc for Android (bionic) has had accept4() since API level 21:
https://android.googlesource.com/platform/bionic/+/refs/heads/master/docs/status.md

That corresponds to Android 5.0. So should we update the minimum Android API level in CI or just not use accept4() on Android?

@de-vri-es
Copy link
Contributor Author

Submitted #78601 to see if we can update the Android API level for CI. If not, I'll just remove Android from the list for accept4().

@de-vri-es
Copy link
Contributor Author

rust-lang/libc#1968 got merged. Once it is released I'll update this PR.

@m-ou-se
Copy link
Member

m-ou-se commented Nov 6, 2020

I suppose you could also just use syscall(SYS_accept4, ..) directly on Android then, until the libc issues are resolved.

@de-vri-es
Copy link
Contributor Author

Good point, applied the same workaround here :)

@de-vri-es de-vri-es force-pushed the bsd-cloexec branch 2 times, most recently from fdb62a1 to bb3bd1b Compare November 6, 2020 12:52
@m-ou-se
Copy link
Member

m-ou-se commented Nov 6, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Nov 6, 2020

📌 Commit 3bee37c has been approved by m-ou-se

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 6, 2020
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Nov 7, 2020
Use SOCK_CLOEXEC and accept4() on more platforms.

This PR enables the use of `SOCK_CLOEXEC` and `accept4` on more platforms.

-----

Android uses the linux kernel, so it should also support it.

DragonflyBSD introduced them in 4.4 (December 2015):
https://www.dragonflybsd.org/release44/

FreeBSD introduced them in 10.0 (January 2014):
https://wiki.freebsd.org/AtomicCloseOnExec

Illumos introduced them in a commit in April 2013, not sure when it was released. It is quite possible that is has always been in Illumos:
illumos/illumos-gate@5dbfd19
https://illumos.org/man/3socket/socket
https://illumos.org/man/3socket/accept4

NetBSD introduced them in 6.0 (Oktober 2012) and 8.0 (July 2018):
https://man.netbsd.org/NetBSD-6.0/socket.2
https://man.netbsd.org/NetBSD-8.0/accept.2

OpenBSD introduced them in 5.7 (May 2015):
https://man.openbsd.org/socket https://man.openbsd.org/accept
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Nov 7, 2020
Use SOCK_CLOEXEC and accept4() on more platforms.

This PR enables the use of `SOCK_CLOEXEC` and `accept4` on more platforms.

-----

Android uses the linux kernel, so it should also support it.

DragonflyBSD introduced them in 4.4 (December 2015):
https://www.dragonflybsd.org/release44/

FreeBSD introduced them in 10.0 (January 2014):
https://wiki.freebsd.org/AtomicCloseOnExec

Illumos introduced them in a commit in April 2013, not sure when it was released. It is quite possible that is has always been in Illumos:
illumos/illumos-gate@5dbfd19
https://illumos.org/man/3socket/socket
https://illumos.org/man/3socket/accept4

NetBSD introduced them in 6.0 (Oktober 2012) and 8.0 (July 2018):
https://man.netbsd.org/NetBSD-6.0/socket.2
https://man.netbsd.org/NetBSD-8.0/accept.2

OpenBSD introduced them in 5.7 (May 2015):
https://man.openbsd.org/socket https://man.openbsd.org/accept
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Nov 7, 2020
Use SOCK_CLOEXEC and accept4() on more platforms.

This PR enables the use of `SOCK_CLOEXEC` and `accept4` on more platforms.

-----

Android uses the linux kernel, so it should also support it.

DragonflyBSD introduced them in 4.4 (December 2015):
https://www.dragonflybsd.org/release44/

FreeBSD introduced them in 10.0 (January 2014):
https://wiki.freebsd.org/AtomicCloseOnExec

Illumos introduced them in a commit in April 2013, not sure when it was released. It is quite possible that is has always been in Illumos:
illumos/illumos-gate@5dbfd19
https://illumos.org/man/3socket/socket
https://illumos.org/man/3socket/accept4

NetBSD introduced them in 6.0 (Oktober 2012) and 8.0 (July 2018):
https://man.netbsd.org/NetBSD-6.0/socket.2
https://man.netbsd.org/NetBSD-8.0/accept.2

OpenBSD introduced them in 5.7 (May 2015):
https://man.openbsd.org/socket https://man.openbsd.org/accept
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Nov 7, 2020
Use SOCK_CLOEXEC and accept4() on more platforms.

This PR enables the use of `SOCK_CLOEXEC` and `accept4` on more platforms.

-----

Android uses the linux kernel, so it should also support it.

DragonflyBSD introduced them in 4.4 (December 2015):
https://www.dragonflybsd.org/release44/

FreeBSD introduced them in 10.0 (January 2014):
https://wiki.freebsd.org/AtomicCloseOnExec

Illumos introduced them in a commit in April 2013, not sure when it was released. It is quite possible that is has always been in Illumos:
illumos/illumos-gate@5dbfd19
https://illumos.org/man/3socket/socket
https://illumos.org/man/3socket/accept4

NetBSD introduced them in 6.0 (Oktober 2012) and 8.0 (July 2018):
https://man.netbsd.org/NetBSD-6.0/socket.2
https://man.netbsd.org/NetBSD-8.0/accept.2

OpenBSD introduced them in 5.7 (May 2015):
https://man.openbsd.org/socket https://man.openbsd.org/accept
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 8, 2020
Rollup of 19 pull requests

Successful merges:

 - rust-lang#76097 (Stabilize hint::spin_loop)
 - rust-lang#76227 (Stabilize `Poll::is_ready` and `is_pending` as const)
 - rust-lang#78065 (make concurrency helper more pleasant to read)
 - rust-lang#78570 (Remove FIXME comment in print_type_sizes ui test suite)
 - rust-lang#78572 (Use SOCK_CLOEXEC and accept4() on more platforms.)
 - rust-lang#78658 (Add a tool to run `x.py` from any subdirectory)
 - rust-lang#78706 (Fix run-make tests running when LLVM is disabled)
 - rust-lang#78728 (Constantify `UnsafeCell::into_inner` and related)
 - rust-lang#78775 (Bump Rustfmt and RLS)
 - rust-lang#78788 (Correct unsigned equivalent of isize to be usize)
 - rust-lang#78811 (Make some std::io functions `const`)
 - rust-lang#78828 (use single char patterns for split() (clippy::single_char_pattern))
 - rust-lang#78841 (Small cleanup in `TypeFoldable` derive macro)
 - rust-lang#78842 (Honor the rustfmt setting in config.toml)
 - rust-lang#78843 (Less verbose debug logging from inlining integrator)
 - rust-lang#78852 (Convert a bunch of intra-doc links)
 - rust-lang#78860 (rustc_resolve: Use `#![feature(format_args_capture)]`)
 - rust-lang#78861 (typo and formatting)
 - rust-lang#78865 (Don't fire `CONST_ITEM_MUTATION` lint when borrowing a deref)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit eef9951 into rust-lang:master Nov 8, 2020
@rustbot rustbot added this to the 1.49.0 milestone Nov 8, 2020
aruediger pushed a commit to Actyx/wsrpc that referenced this pull request Mar 12, 2021
With Rust 1.49.0, accepting incoming connections on tcp sockets failed
in different ways:
Starting with Android Oreo (8), Android started using a seccomp based
filter approach to syscalls, explicitly allowing syscalls, see
https://android-developers.googleblog.com/2017/07/seccomp-filter-in-android-o.html.
https://android.googlesource.com/platform/bionic.git/+/master/libc/SYSCALLS.TXT
enumerates the allowed syscalls.
rust-lang/rust#78572 refactored the way
`std::net::TcpListener` accepts incoming connections on tcp sockets.
With the seccomp profile above, doing a generic syscall will result in a
panic:
```
[..]
02-22 13:14:23.288  6015  6041 F my.app.DEBUG: signal 31 (SIGSYS), code
1 (SYS_SECCOMP), fault addr --------
02-22 13:14:23.288  6015  6041 F my.app.DEBUG: Cause: seccomp prevented
call to disallowed x86 system call 364
02-22 13:14:23.289  6015  6041 F my.app.DEBUG: Abort message: 'Fatal
signal 31 (SIGSYS), code 1 (SYS_SECCOMP) in tid 4784 (tokio-runtime-w),
pid 4735 (ground_services)'
```

On top of that, I found that older versions of Android, such as Android
6 (our Zebra ET50), will return Function not implemented (os error 38)
for this syscall.  My tests showed that this only happens on x86,
although I can't explain why. Relevant strace:
```
[pid 10918] syscall_364(0x34, 0x9d5c9cf8, 0x9d5c9ca0, 0x80800,
0x9fda9dc8, 0x9fda9dc8 <unfinished ...>
[pid 10918] <... syscall_364 resumed> ) = -1 (errno 38)
```

I have tested this with both real devices as well as Android emulators.

We have been using the `async-io` based `libp2p::tcp::TcpConfig` so far,
which used `std::net::TcpListener` under the hood. This commit also
switches to using `libp2p::tcp::TokioTcpConfig`. Now, tokio uses mio,
which doesn't use `std::net::TcpListener` but raw sockets directly.
Recently, a workaround for the erroneous behaviour described above was
merged to mio, which is still pending to be released on crates.io
(tokio-rs/mio#1462).  Once tokio uses the
updated mio version, we should move back to the crates.io provided
version.

For tracking the issue in `std::net::TcpListener`, I created
rust-lang/rust#82400.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-android Operating system: Android O-dragonfly Operating system: DragonFly BSD O-freebsd Operating system: FreeBSD O-netbsd Operating system: NetBSD O-openbsd Operating system: OpenBSD S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants