util: Fix GUIX build with syscall sandbox #23178

pull laanwj wants to merge 2 commits into bitcoin:master from laanwj:2021-10-guix-linux-sandbox changing 1 files +18 −8

laanwj commented at 10:11 PM on October 4, 2021: member
Looks like we've broke the GUIX build in #20487. This attempts to fix it:
- Define __NR_statx __NR_getrandom __NR_membarrier as some kernel headers lack them, and it's important to have the same profile independent on what kernel is used for building.
- Define SECCOMP_RET_KILL_PROCESS as it isn't defined in the headers.
laanwj added the label Build system on Oct 4, 2021
laanwj added the label Utils/log/libs on Oct 4, 2021
laanwj requested review from practicalswift on Oct 4, 2021
laanwj requested review from dongcarl on Oct 4, 2021
laanwj commented at 10:21 PM on October 4, 2021: member
Thinking of it, I'm not sure this is the right solution. statx is not directly used by us, but by libc. What if the resulting binary is run against a more recent version of libc (remember, we link libc dynamically), it will use the system call but not be allowed to, so fail?

Might be better to do
```
#ifndef __NR_statx
#define __NR_statx 332
#endif
```
…instead And the same for __NR_getrandom and __NR_membarrier.

Edit: pushed this new solution

util: Define SECCOMP_RET_KILL_PROCESS if not provided by the headers

Define `SECCOMP_RET_KILL_PROCESS` as it isn't defined in the headers, as
is the case for the GUIX build on this platform.

8289d19ea5

laanwj force-pushed on Oct 5, 2021
MarcoFalke added the label DrahtBot Guix build requested on Oct 5, 2021
laanwj force-pushed on Oct 5, 2021
MarcoFalke referenced this in commit c79d9fb2f6 on Oct 5, 2021
practicalswift approved

practicalswift commented at 9:54 AM on October 5, 2021: contributor

cr ACK 1685d1221e7e605ff073df94e223420691afa079

This might be helpful for fellow reviewers:

$ grep -E '(statx|getrandom|membarrier)$' linux/arch/x86/entry/syscalls/syscall_64.tbl
318     common  getrandom               sys_getrandom
324     common  membarrier              sys_membarrier
332     common  statx                   sys_statx
$ grep SECCOMP_RET_KILL_PROCESS linux/include/uapi/linux/seccomp.h
#define SECCOMP_RET_KILL_PROCESS 0x80000000U /* kill the process */

MarcoFalke commented at 10:09 AM on October 5, 2021: member

Can remove this line now?

https://github.com/bitcoin/bitcoin/blob/1685d1221e7e605ff073df94e223420691afa079/src/util/syscall_sandbox.cpp#L179

practicalswift commented at 10:20 AM on October 5, 2021: contributor

@MarcoFalke Yes, I think these two no longer need to be conditional:

#if defined(__NR_getrandom)
    {__NR_getrandom, "getrandom"},
#endif // defined(__NR_getrandom)
…
#if defined(__NR_membarrier)
    {__NR_membarrier, "membarrier"},
#endif // defined(__NR_membarrier)

laanwj commented at 12:02 PM on October 5, 2021: member

Can remove this line now?

I'm still not decided what I want to do with that table, but i i's supposed to be more or less platform-independent (see also discussion here: #20487 (review) ). I left the conditional like this for platforms that really don't have the getrandom/membarrier call.

util: Make sure syscall numbers used in profile are defined

Define the following syscall numbers for x86_64, so that the profile
will be the same no matter what kernel is built against, including
kernels that don't have `__NR_statx`:
```c++
 #define __NR_statx 332
 #define __NR_getrandom 318
 #define __NR_membarrier 324
```

2d0279987e

laanwj force-pushed on Oct 5, 2021
laanwj commented at 12:43 PM on October 5, 2021: member

Anyhow, removed them and force-pushed. I agree it's also somewhat confusing and they can always be added again.
practicalswift commented at 12:56 PM on October 5, 2021: contributor

cr ACK 2d0279987ef04edda5f61c171768b9527cc936cc

Thanks for quickly resolving this!

practicalswift commented at 2:15 PM on October 5, 2021: contributor

FWIW this is how minijail (a sandboxing and containment tool used in Chrome OS and Android) handles this:

/* Ideally minijail is compiled against a modern libc, which has modern copies
 * of Linux uapi for ioctls, and unistd.h for syscalls. However, sometimes this
 * isn't possible - such as when building with the Android host toolchain - so
 * locally define the system calls in use in active seccomp policy files.
 * This UAPI is taken from sanitized bionic headers.
 */

#ifndef __NR_copy_file_range
#ifdef __x86_64__
#define __NR_copy_file_range 326
#elif __i386__
#define __NR_copy_file_range 377
#elif __arm64__
#define __NR_copy_file_range 285
#endif
#endif /* __NR_copy_file_range */
…

https://github.com/google/minijail/blob/6aa0392f3a948c09eb8305650f5d3067e94b25af/gen_syscalls-inl.h#L8-L23

laanwj commented at 2:24 PM on October 5, 2021: member

Thanks—we could do the same when we support sandboxing on multiple platforms.
sidhujag referenced this in commit 6360e632d2 on Oct 5, 2021
laanwj merged this on Oct 5, 2021
laanwj closed this on Oct 5, 2021
sidhujag referenced this in commit 5c90f6b9a3 on Oct 5, 2021

DrahtBot commented at 7:59 AM on October 6, 2021: member

Guix builds

File	commit 816e15ee81a2029cde6b4f9fe6fb93e75478c903<br>(master)	commit 0afd60bcfb0a7f853210eb2c3af94283ba182b8d<br>(master and this pull)
*.tar.gz	`1350388139b5ea3d...`	`5ddf35dfc76a4aa8...`
guix_build.log	`d856056de4d6bf62...`	`e61e6e6cb6397634...`
SHA256SUMS.part		`d04222fdf9f985fb...`
*-aarch64-linux-gnu-debug.tar.gz		`e6dc2147b545c6f2...`
*-aarch64-linux-gnu.tar.gz		`3c0982848ecc3b2a...`
*-arm-linux-gnueabihf-debug.tar.gz		`00746f713efec4cb...`
*-arm-linux-gnueabihf.tar.gz		`d20f0d3455324cd3...`
*-osx-unsigned.dmg		`beb1174d759bf397...`
*-osx-unsigned.tar.gz		`fc509bd7c1dc487c...`
*-osx64.tar.gz		`531609737c6de7b0...`
*-powerpc64-linux-gnu-debug.tar.gz		`0f9acb5de2c66a4c...`
*-powerpc64-linux-gnu.tar.gz		`bc25c8fd07d634e1...`
*-powerpc64le-linux-gnu-debug.tar.gz		`c1264831b1aabf6c...`
*-powerpc64le-linux-gnu.tar.gz		`32becb071c8b7059...`
*-riscv64-linux-gnu-debug.tar.gz		`4289c2afd7fb0692...`
*-riscv64-linux-gnu.tar.gz		`f62a44650044edcf...`
*-win-unsigned.tar.gz		`8ab05ff22cc2a254...`
*-win64-debug.zip		`48cee67be9779712...`
*-win64-setup-unsigned.exe		`d15e7c917e6db189...`
*-win64.zip		`7acd902e319924a5...`
*-x86_64-linux-gnu-debug.tar.gz		`a4b29211c64b4f10...`
*-x86_64-linux-gnu.tar.gz		`65163bd030110994...`
guix_build.log.diff		`ed83cc9d8254dede...`