On the musl profiles we current apply a dirty patch to get sandbox built: https://gitweb.gentoo.org/proj/hardened-dev.git/tree/sys-apps/sandbox/files/sandbox-2.6-musl.patch?h=musl Below will be patches for two issues. After that one issue remains, which is LARGEFILE support. musl has LFS support by default. (off_t is always 64 bit.) If LFS64 support is wanted off64_t is #define'd to off_t, open64 to open, ... For binary compatibility with glibc the LFS64 symbols exist as weak aliases, e.g. open64 is a weak alias to open. Because they see the open64 symbol in libc.so the sandbox scripts add this symbol to the symbols being overriden. This is however unecessary and also does not work because of the #defines. The proper fix would be for the sandbox scripts to record the symbol values from the readelf output in order to detect weak aliases. (and then use the information in order not to overwrite a strong symbol and a weak alias of it.) However my awk skills are lacking... Reproducible: Always
Created attachment 403010 [details, diff] Don't use enum __ptrace_request
Created attachment 403012 [details, diff] Protect against redefinition of ptrace_peeksiginfo_args
Current version (2.6-r999) of sandbox fails to build against musl-1.1.9 due to changes in function visibility between 1.1.8 and 1.1.9. Previously most functions had visibility DEFAULT but they are now defaulting to PROTECTED instead, see http://git.musl-libc.org/cgit/musl/commit/?id=de2b67f8d41e08caa56bf6540277f6561edb647f
Created attachment 403814 [details] Build.log of sandbox against musl-1.1.9 Build fails due to gen_symbol_version_map.awk not finding any global default visibility symbols
Created attachment 404754 [details, diff] Somewhat hacky patch fixing the remaining issues
(In reply to Felix Janda from comment #5) > Created attachment 404754 [details, diff] [details, diff] > Somewhat hacky patch fixing the remaining issues I just saw this patch of yours before I went ahead and fixed sandbox to work wit 1.1.10. The gen_symbol_version_map.awk script I fixed by adding a check for PROTECTED visibility rather than removing the DEFAULT check. This is safer in case you don't have a stripped libc.so. I don't understand why you added "&& !defined(open64)" Why is this needed? I also don't get why you added the check for "$5 != WEAK" to gen_symbol_header.awk. Are you sure that's correct?
(In reply to Anthony Basile from comment #6) > (In reply to Felix Janda from comment #5) > > Created attachment 404754 [details, diff] [details, diff] [details, diff] > > Somewhat hacky patch fixing the remaining issues > > I just saw this patch of yours before I went ahead and fixed sandbox to work I mean I pushed my fix before I saw your patch. Since I'm building the new stages based on my fix, I want to make sure I haven't missed anything.
The patches of this bug are supposed to replace the sandbox-2.6-musl.patch (and be upstreamed at some point). When you try to compile without sandbox-2.6-musl.patch you will see what the other parts of Attachment 404754 [details, diff] are for. The patch is very likely wrong but maybe something along the lines can be done.
*** Bug 551624 has been marked as a duplicate of this bug. ***
sanbox-2.10 is being stabilized (bug 571308) and needs the additional patch for gen_symbol_version_map.awk. @Anthony: You meant changing the condition to the following? if ($4 != "FUNC" || $5 == "LOCAL" || ($6 != "DEFAULT" && $6 != "PROTECTED"))
(In reply to Felix Janda from comment #10) > sanbox-2.10 is being stabilized (bug 571308) and needs the additional > patch for gen_symbol_version_map.awk. > > @Anthony: You meant changing the condition to the following? > > if ($4 != "FUNC" || $5 == "LOCAL" || ($6 != "DEFAULT" && $6 != "PROTECTED")) yeah i'll try to take care of sandbox-2.10 soon.
What's the status here? Is there anything still needing to be applied on sandbox end?
Still doesn't build. musl overlay currently uses the patches [1], [2]. [1]: https://gitweb.gentoo.org/proj/musl.git/tree/sys-apps/sandbox/files/sandbox-2.10-fix-visibility-musl.patch [2]: https://gitweb.gentoo.org/proj/musl.git/tree/sys-apps/sandbox/files/sandbox-2.11-musl.patch
Could you convert them to git format patches with an explanatory commit message? They look black magic to me. Also, at a first glance /* */ around #include looks very wrong.
Created attachment 496772 [details, diff] Patch 1
Created attachment 496774 [details, diff] Patch 2
Created attachment 496776 [details, diff] Patch 3 (hack) This patch fixes the following problem for musl while likely breaking other stuff: The awk script gen_symbol_header.awk creates a header file symbols.h with wrappers for all libc functions sandbox wants to intercept. These functions can be found in symbols.h.in, for example open and open64. In glibc, these are different symbols. In musl however these are the same symbol, and the musl headers even #define open64 as open. The end result are conflicting definitions in the case of musl. The patch makes the awk script leave out the definitions of all functions ending in 64 in the case of a libc without symbol versioning. This should work for glibc and musl, however it is very unlikely to work for other libcs.
Created attachment 496778 [details, diff] Patch 4 This makes the code conform to the above comment, it fixes compilation with musl but I'm not sure whether it is correct.
Instead of the hack in Patch 2, it appears that the header <linux/ptrace.h> could also be removed. But since the header headers.h is used everywhere, it is difficult to see whether <linux/ptrace.h> is really unecessary.
Created attachment 580598 [details, diff] 0001-libsandbox-trace.c-tweak-ptrace-command-type-for-mus.patch 0001-libsandbox-trace.c-tweak-ptrace-command-type-for-mus.patch fixes ptrace() proto build failure.
Created attachment 580600 [details, diff] 0002-scripts-gen_symbol_header.awk-undefine-libc-symbol-a.patch 0002-scripts-gen_symbol_header.awk-undefine-libc-symbol-a.patch fixes symbol clobbers from libc's #defines.
With 0001-libsandbox-trace.c-tweak-ptrace-command-type-for-mus.patch 0002-scripts-gen_symbol_header.awk-undefine-libc-symbol-a.patch patches applied sanbox's test suite survives on amd64/glibc (all tests pass) and is reasoably healthy on amd64/musl (one test fails): ... 82: utimensat/2 ok 83: utimensat/3 FAILED (utimensat.at:3) 86: utimensat_static/2 ok 85: utimensat_static/1 ok 87: vfork/1 ok 59: script/5 ok 84: utimensat/4 ok ## ------------- ## ## Test results. ## ## ------------- ## ERROR: All 87 tests were run, 1 failed unexpectedly. ## -------------------------- ## ## testsuite.log was created. ## ## -------------------------- ##
The commit message in second patch seems to be missing something after: > Avoid libc's symbol rename via #define. musl defines aliases as: (I'm guessing '#' got treated as comment)
Created attachment 580730 [details, diff] v2-0002-scripts-gen_symbol_header.awk-undefine-libc-symbo.patch
(In reply to Michał Górny from comment #23) > The commit message in second patch seems to be missing something after: > > > Avoid libc's symbol rename via #define. musl defines aliases as: > > (I'm guessing '#' got treated as comment) Good catch! Added a bit of indent to make sure #defines stay in the commit message in v2-0002-scripts-gen_symbol_header.awk-undefine-libc-symbo.patch
LGTM. Feel free to push them.
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/proj/sandbox.git/commit/?id=ee52c156905c419323865e7cdd2deb46040f0c8a commit ee52c156905c419323865e7cdd2deb46040f0c8a Author: Sergei Trofimovich <slyfox@gentoo.org> AuthorDate: 2019-06-23 20:50:54 +0000 Commit: Sergei Trofimovich <slyfox@gentoo.org> CommitDate: 2019-06-25 06:39:27 +0000 scripts/gen_symbol_header.awk: undefine libc symbol aliases Avoid libc's symbol rename via #define. musl defines aliases as: #define mkstemp64 mkstemp #define mkstemps64 mkstemps This causes libsandbox's aliases to clash with one another, like mkstemp and mkstemp64. The change does not break glibc and restores musl support. Bug: https://bugs.gentoo.org/549108 Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org> scripts/gen_symbol_header.awk | 4 ++++ 1 file changed, 4 insertions(+) https://gitweb.gentoo.org/proj/sandbox.git/commit/?id=ed72f5d07f627464a95ab377cd101d90d4d10c7d commit ed72f5d07f627464a95ab377cd101d90d4d10c7d Author: Sergei Trofimovich <slyfox@gentoo.org> AuthorDate: 2019-06-23 20:48:26 +0000 Commit: Sergei Trofimovich <slyfox@gentoo.org> CommitDate: 2019-06-25 06:39:05 +0000 libsandbox/trace.c: tweak ptrace command type for musl glibc defines ptrace as: long ptrace(enum __ptrace_request request, pid_t pid, void *addr, void *data); musl defines ptrace as: long ptrace(int, ...); This causes build failure in for of: ../../sandbox-2.17/libsandbox/trace/linux/x86_64.c: In function 'trace_set_ret': ../../sandbox-2.17/libsandbox/trace/linux/x86_64.c:99:2: error: type of formal parameter 1 is incomplete trace_set_regs(regs); ^~~~~~~~~~~~~~ Let's clobber to 'int' lowest common denominator. Bug: https://bugs.gentoo.org/549108 Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org> libsandbox/trace.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
> 83: utimensat/3 FAILED (utimensat.at:3) Asted musl@ about that failure. Apparently passing NULL path to utimensat is not a specified behaviour anywhere. But glibc happens to guard against it with EINVAL. musl does not and does not plan to: https://www.openwall.com/lists/musl/2019/06/25/1 We'll need to either remove the test or make it glibc-specific. I'll try to make it glibc-specific. After that we can declare basic musl compatibility.
Created attachment 581128 [details, diff] 0001-tests-disable-utimensat-3-on-linux-musl.patch
(In reply to Sergei Trofimovich from comment #29) > Created attachment 581128 [details, diff] [details, diff] > 0001-tests-disable-utimensat-3-on-linux-musl.patch This also LGTM.
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/proj/sandbox.git/commit/?id=604927f331829f971d3a32c2e67e0ad5ce3d8ee4 commit 604927f331829f971d3a32c2e67e0ad5ce3d8ee4 Author: Sergei Trofimovich <slyfox@gentoo.org> AuthorDate: 2019-06-27 09:09:56 +0000 Commit: Sergei Trofimovich <slyfox@gentoo.org> CommitDate: 2019-06-27 09:09:56 +0000 tests: disable utimensat-3 on *-linux-musl x86_64-gentoo-linux-musl fails a single test: 83: utimensat/3 FAILED (utimensat.at:3) The test checks if sandbox does not crash when utimensat(<filefd>, NULL, NULL, 0) is called. The behaviour is not specified by POSIX but glibc returns EINVAL for such a case. Thus the test behaves differently on varius libs. https://www.openwall.com/lists/musl/2019/06/25/1 has a conversation with musl upstream. The change restricts test down to glibc targets. Bug: https://bugs.gentoo.org/549108 Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org> tests/atlocal.in | 1 + tests/utimensat-3.sh | 11 +++++++++++ 2 files changed, 12 insertions(+)
The bug has been closed via the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=44a8ff0d8ea8e992e7956f99f9665f6d7439b84a commit 44a8ff0d8ea8e992e7956f99f9665f6d7439b84a Author: Sergei Trofimovich <slyfox@gentoo.org> AuthorDate: 2019-07-12 06:48:30 +0000 Commit: Sergei Trofimovich <slyfox@gentoo.org> CommitDate: 2019-07-12 06:48:46 +0000 sys-apps/sandbox: bump up to 2.18, bug #549108 The main change from 2.17 is basic musl support. Closes: https://bugs.gentoo.org/549108 Package-Manager: Portage-2.3.69, Repoman-2.3.16 Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org> sys-apps/sandbox/Manifest | 1 + sys-apps/sandbox/files/musl.patch | 42 ++++++++++++++++++++ sys-apps/sandbox/sandbox-2.18.ebuild | 74 ++++++++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+)
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/proj/sandbox.git/commit/?id=b8a6ddb6fc1832abc1b396f285b864858bb18026 commit b8a6ddb6fc1832abc1b396f285b864858bb18026 Author: Sam James <sam@gentoo.org> AuthorDate: 2024-11-11 10:33:42 +0000 Commit: Sam James <sam@gentoo.org> CommitDate: 2024-11-11 22:47:01 +0000 tests: handle traps on NULL input for utimensat When building with -fisolate-erroneous-paths-attribute, GCC turns the call to utimensat w/ a NULL arg into a trap because of its nonnull attribute(s). Workaround that in the test. Bug: https://bugs.gentoo.org/549108 Signed-off-by: Sam James <sam@gentoo.org> tests/utimensat-0.c | 5 +++++ 1 file changed, 5 insertions(+)
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/proj/sandbox.git/commit/?id=d37d82ff00bc2e4b0a10b50679f5233e0d056cb7 commit d37d82ff00bc2e4b0a10b50679f5233e0d056cb7 Author: Sam James <sam@gentoo.org> AuthorDate: 2024-11-11 10:33:42 +0000 Commit: Sam James <sam@gentoo.org> CommitDate: 2024-11-11 22:48:36 +0000 tests: handle traps on NULL input for utimensat When building with -fisolate-erroneous-paths-attribute, GCC turns the call to utimensat w/ a NULL arg into a trap because of its nonnull attribute(s). Workaround that in the test. Bug: https://bugs.gentoo.org/549108 Signed-off-by: Sam James <sam@gentoo.org> (cherry picked from commit b8a6ddb6fc1832abc1b396f285b864858bb18026) tests/utimensat-0.c | 5 +++++ 1 file changed, 5 insertions(+)
