util: add linkat to syscall sandbox (AllowFileSystem) #24659
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
|
cr ACK 9809db3 but might be good to test |
Member
|
Should we allow regular |
Member
Author
Yes, if someone reports an issue with it. I don't think we want/need to preemptively allow syscalls. |
Member
|
We don't usually leave known bugs until someone reports them... |
Member
|
Yeah, I think it is fine to put in both. |
Member
Author
Sure, but this isn't a known bug yet. If we want it to be one, can you elaborate more than "my manpage says maybe this is needed for some older kernels". Which manpage? Which older kernel (versions)? Which versions of glibc perform the fallback-ing? Do we support them? Can you reproduce the bug? I'm not really interested in adding exceptions to our syscall sandbox based on hypotheticals derived from a possibly irrelevant (to our project & runtime back-compatibilities) sentence in a manpage. Especially if the problem hasn't actually been run into in the wild. |
hebasto
reviewed
Mar 26, 2022
| allowed_syscalls.insert(__NR_getcwd); // get current working directory | ||
| allowed_syscalls.insert(__NR_getdents); // get directory entries | ||
| allowed_syscalls.insert(__NR_getdents64); // get directory entries | ||
| allowed_syscalls.insert(__NR_linkat); // create relative to a directory file descriptor |
Member
There was a problem hiding this comment.
Tested on Ubuntu 22.04:
Suggested change
| allowed_syscalls.insert(__NR_linkat); // create relative to a directory file descriptor | |
| allowed_syscalls.insert(__NR_inotify_rm_watch);// remove an existing watch from an inotify instance | |
| allowed_syscalls.insert(__NR_linkat); // create relative to a directory file descriptor |
Contributor
Member
|
Going to merge this, since it is tested and reviewed. Maybe issues on other OS can be fixed in a separate commit? |
Member
|
Backported to v23.0 in #24512 |
Merged
fanquake
added a commit
to bitcoin-core/gui
that referenced
this pull request
Mar 30, 2022
…ndbox (AllowFileSystem) f05a4cd util: Add inotify_rm_watch to syscall sandbox (AllowFileSystem) (Hennadii Stepanov) Pull request description: This PR fixes the current master (3297f5c) when running `bitcoin-qt` on Ubuntu 22.04 and quitting: ``` $ ./src/qt/bitcoin-qt -signet -sandbox=log-and-abort Warning: Ignoring XDG_SESSION_TYPE=wayland on Gnome. Use QT_QPA_PLATFORM=wayland to run on Wayland anyway. ERROR: The syscall "inotify_rm_watch" (syscall number 255) is not allowed by the syscall sandbox in thread "main". Please report. terminate called without an active exception Aborted (core dumped) ``` Also see bitcoin/bitcoin#24659 (comment) ACKs for top commit: fanquake: ACK f05a4cd - checked that qt is using this in it's filesystem watcher code. Tree-SHA512: 9c7920a25422cd3a040bc1cbc487c12c3dc2b91358c3757f1030d6a1ff12c18c688a8e5b7466f683da88a5e4f5f15d442975660022d706e47021253c24c58f4a
jonatack
pushed a commit
to jonatack/bitcoin
that referenced
this pull request
Mar 31, 2022
Github-Pull: bitcoin#24659 Rebased-From: 9809db3
fanquake
added a commit
that referenced
this pull request
Mar 31, 2022
174af33 util: Add inotify_rm_watch to syscall sandbox (AllowFileSystem) (Hennadii Stepanov) ded10fe build: Fix Boost.Process test for Boost 1.78 (Hennadii Stepanov) 26c2f23 build: Fix Boost.Process detection on macOS arm64 (Hennadii Stepanov) 85f85c7 util: add linkat to syscall sandbox (AllowFileSystem) (fanquake) eaa0419 contrib: fix signet miner (sighash mismatch) (Sebastian Falbesoner) 235b042 rpc: Exclude descriptor when address is excluded (MarcoFalke) b05a59b ci: Temporarily use clang-13 to work around clang-14 TSan bug (MarcoFalke) 65b9667 doc, init: add links to doc/cjdns.md (Jon Atack) 7a553d4 doc: update i2p.md with cjdns, improve local addresses section (Jon Atack) 4148396 doc: update tor.md with cjdns and getnodeaddresses, fix tor grep, (Jon Atack) 4690e8a doc: create initial doc/cjdns.md for cjdns how-to documentation (Jon Atack) 5d24f61 Clarify in -maxtimeadjustment that only outbound peers influence time data (Jon Atack) b1646f1 test: set segwit height back to 0 on regtest (Martin Zumsande) ef6a37b rpc: rename getdeploymentinfo status-next to status_next (Jon Atack) 2a6fcf9 init, doc: improve -onlynet help and tor/i2p documentation (Jon Atack) Pull request description: Backport the following to 23.x: - #24468 - #24528 - #24527 - #24609 - #24555 - #24663 - #24572 - #24636 - #24553 - #24659 - #24521 - #24523 - #24690 - #24710 Possibly also: - #24579 - #24691 ACKs for top commit: laanwj: List-of-commits ACK 174af33, I think we should merge this and move forward with rc3.. hebasto: ACK 174af33 Tree-SHA512: 5a493e1652b780b527767d6ca9e67012abd2fa5573496e85e0d8aa4bed3eb332bfcd72610b8dfb954ff274d42450623233c96c479de2085b9c8344ba5abf1935
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Apr 2, 2022
…stem) 9809db3 util: add linkat to syscall sandbox (AllowFileSystem) (fanquake) Pull request description: Should fix bitcoin#24536. ACKs for top commit: MarcoFalke: cr ACK 9809db3 Rspigler: Tested ACK (commit 9809db3) - this fixes bitcoin#24536 Tree-SHA512: 2642f7dfa806e166fb32639a29b509b2edc8b919516c1f12430fc96f9887952395e157d71ef99fbaef8f7bcce1920530c24ecbce605b8a374b05d586f1f22a24
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Apr 3, 2022
…llowFileSystem) f05a4cd util: Add inotify_rm_watch to syscall sandbox (AllowFileSystem) (Hennadii Stepanov) Pull request description: This PR fixes the current master (3297f5c) when running `bitcoin-qt` on Ubuntu 22.04 and quitting: ``` $ ./src/qt/bitcoin-qt -signet -sandbox=log-and-abort Warning: Ignoring XDG_SESSION_TYPE=wayland on Gnome. Use QT_QPA_PLATFORM=wayland to run on Wayland anyway. ERROR: The syscall "inotify_rm_watch" (syscall number 255) is not allowed by the syscall sandbox in thread "main". Please report. terminate called without an active exception Aborted (core dumped) ``` Also see bitcoin#24659 (comment) ACKs for top commit: fanquake: ACK f05a4cd - checked that qt is using this in it's filesystem watcher code. Tree-SHA512: 9c7920a25422cd3a040bc1cbc487c12c3dc2b91358c3757f1030d6a1ff12c18c688a8e5b7466f683da88a5e4f5f15d442975660022d706e47021253c24c58f4a
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Should fix #24536.