|Summary:||sys-apps/sandbox: build with musl libc|
|Product:||Portage Development||Reporter:||Felix Janda <felix.janda>|
|Component:||Sandbox||Assignee:||Sandbox Maintainers <sandbox>|
|Severity:||normal||CC:||fredric.miscmail, gentoo, gentoo, mgorny, tsmksubc|
|Package list:||Runtime testing required:||---|
|Bug Depends on:|
Don't use enum __ptrace_request
Protect against redefinition of ptrace_peeksiginfo_args
Build.log of sandbox against musl-1.1.9
Somewhat hacky patch fixing the remaining issues
Patch 3 (hack)
Description Felix Janda 2015-05-10 21:14:32 UTC
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
Comment 1 Felix Janda 2015-05-10 21:15:25 UTC
Created attachment 403010 [details, diff] Don't use enum __ptrace_request
Comment 2 Felix Janda 2015-05-10 21:17:38 UTC
Created attachment 403012 [details, diff] Protect against redefinition of ptrace_peeksiginfo_args
Comment 3 Fredric Johansson 2015-05-23 15:27:36 UTC
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
Comment 4 Fredric Johansson 2015-05-23 15:30:55 UTC
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
Comment 5 Felix Janda 2015-06-07 18:36:11 UTC
Created attachment 404754 [details, diff] Somewhat hacky patch fixing the remaining issues
Comment 6 Anthony Basile 2015-06-08 19:58:18 UTC
(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?
Comment 7 Anthony Basile 2015-06-08 20:05:22 UTC
(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.
Comment 8 Felix Janda 2015-06-08 20:38:01 UTC
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.
Comment 9 Anthony Basile 2015-06-10 10:41:14 UTC
*** Bug 551624 has been marked as a duplicate of this bug. ***
Comment 10 Felix Janda 2016-01-16 17:27:09 UTC
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"))
Comment 11 Anthony Basile 2016-01-22 17:58:07 UTC
(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.
Comment 12 Michał Górny 2017-09-26 10:33:19 UTC
What's the status here? Is there anything still needing to be applied on sandbox end?
Comment 13 Felix Janda 2017-09-26 11:42:10 UTC
Still doesn't build. musl overlay currently uses the patches , . : https://gitweb.gentoo.org/proj/musl.git/tree/sys-apps/sandbox/files/sandbox-2.10-fix-visibility-musl.patch : https://gitweb.gentoo.org/proj/musl.git/tree/sys-apps/sandbox/files/sandbox-2.11-musl.patch
Comment 14 Michał Górny 2017-09-26 12:49:34 UTC
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.
Comment 17 Felix Janda 2017-09-27 22:37:08 UTC
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.
Comment 18 Felix Janda 2017-09-27 22:39:17 UTC
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.
Comment 19 Felix Janda 2017-09-27 22:41:06 UTC
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.
Comment 20 Sergei Trofimovich (RETIRED) 2019-06-23 20:53:18 UTC
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.
Comment 21 Sergei Trofimovich (RETIRED) 2019-06-23 20:53:53 UTC
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.
Comment 22 Sergei Trofimovich (RETIRED) 2019-06-23 20:58:44 UTC
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. ## ## -------------------------- ##
Comment 23 Michał Górny 2019-06-24 09:49:10 UTC
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)
Comment 24 Sergei Trofimovich (RETIRED) 2019-06-24 21:00:54 UTC
Created attachment 580730 [details, diff] v2-0002-scripts-gen_symbol_header.awk-undefine-libc-symbo.patch
Comment 25 Sergei Trofimovich (RETIRED) 2019-06-24 21:01:42 UTC
(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
Comment 26 Michał Górny 2019-06-25 06:09:11 UTC
LGTM. Feel free to push them.
Comment 27 Larry the Git Cow 2019-06-25 06:42:12 UTC
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 <email@example.com> AuthorDate: 2019-06-23 20:50:54 +0000 Commit: Sergei Trofimovich <firstname.lastname@example.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 <email@example.com> 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 <firstname.lastname@example.org> AuthorDate: 2019-06-23 20:48:26 +0000 Commit: Sergei Trofimovich <email@example.com> 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 <firstname.lastname@example.org> libsandbox/trace.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
Comment 28 Sergei Trofimovich (RETIRED) 2019-06-27 07:36:34 UTC
> 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.
Comment 29 Sergei Trofimovich (RETIRED) 2019-06-27 09:15:28 UTC
Created attachment 581128 [details, diff] 0001-tests-disable-utimensat-3-on-linux-musl.patch
Comment 30 Michał Górny 2019-06-27 09:40:16 UTC
(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.
Comment 31 Larry the Git Cow 2019-06-27 21:41:34 UTC
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 <email@example.com> AuthorDate: 2019-06-27 09:09:56 +0000 Commit: Sergei Trofimovich <firstname.lastname@example.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 <email@example.com> tests/atlocal.in | 1 + tests/utimensat-3.sh | 11 +++++++++++ 2 files changed, 12 insertions(+)
Comment 32 Larry the Git Cow 2019-07-12 06:48:53 UTC
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 <firstname.lastname@example.org> AuthorDate: 2019-07-12 06:48:30 +0000 Commit: Sergei Trofimovich <email@example.com> 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 <firstname.lastname@example.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(+)