Skip to content

Conversation

@laanwj
Copy link
Member

@laanwj laanwj commented Oct 5, 2021

Make the new syscall sandbox compilable with kernel 4.4.0.
This defines a further syscall constant __NR_copy_file_range to make sure all syscalls used in the profile are available even if not defined in the kernel headers.

Also, make a few syscalls optional in the syscall name table:

  • __NR_pkey_alloc
  • __NR_pkey_free
  • __NR_pkey_mprotect
  • __NR_preadv2
  • __NR_pwritev2

laanwj added 2 commits October 5, 2021 19:35
Put these in `#ifdef` as they are newer syscalls that might not be
defined on all kernels:

     __NR_pkey_alloc
     __NR_pkey_free
     __NR_pkey_mprotect
     __NR_preadv2
     __NR_pwritev2

Thanks to jamesob for reporting.
@laanwj
Copy link
Member Author

laanwj commented Oct 5, 2021

Reported by jamesob on IRC, see https://gist.github.com/jamesob/c46513d41e355a3e6e69f4ff78167c92

I'm still thinking we might want to remove the syscall name table, and replace it with instructions how to look up the number (e.g. as argued here #20487 (comment)). Having this list of syscall constants we don't actually use, besides for error reporting, seems asking for more and more PRs like this.
(another options would be to #ifdef every single one, but this is very verbose… and would make it hard to find actual problems in it)

@jamesob
Copy link
Contributor

jamesob commented Oct 5, 2021

Concept ACK

@laanwj
Copy link
Member Author

laanwj commented Oct 5, 2021

What we also could do is add an autoconf test, and fail the --with-seccomp check below a certain lower bound kernel header version (or easier to detect, when a certain syscall is not in the header).

@practicalswift
Copy link
Contributor

cr ACK ac402e7

Thanks for quickly addressing this!

This might be helpful to fellow reviewers:

$ grep -E 'copy_file_range$' linux/arch/x86/entry/syscalls/syscall_64.tbl
326     common  copy_file_range         sys_copy_file_range

@practicalswift
Copy link
Contributor

practicalswift commented Oct 6, 2021

Some syscall history:

The newest syscall in the LINUX_SYSCALLS map is arch_prctl which was introduced in Linux 4.12 (2017).

These syscalls were introduced from Linux 3.17 (2014) to Linux 4.12 (2017):

  • Linux 3.17 (2014): getrandom, kexec_file_load, membarrier, memfd_create, seccomp
  • Linux 3.18 (2014): bpf
  • Linux 3.19 (2014): execveat
  • Linux 4.0 (2015): -
  • Linux 4.1 (2015): -
  • Linux 4.2 (2015): -
  • Linux 4.3 (2015): userfaultfd
  • Linux 4.4 (2016): mlock2
  • Linux 4.5 (2016): copy_file_range
  • Linux 4.6 (2016): preadv2, pwritev2
  • Linux 4.7 (2016): -
  • Linux 4.8 (2016): pkey_alloc, pkey_free, pkey_mprotect
  • Linux 4.9 (2016): -
  • Linux 4.10 (2017): -
  • Linux 4.11 (2017): statx
  • Linux 4.12 (2017): arch_prctl (arch_prctl has been available for x64_64 since 2.6, x86 since 4.12)

Source: https://man7.org/linux/man-pages/man2/syscalls.2.html

@DrahtBot
Copy link
Contributor

DrahtBot commented Oct 6, 2021

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #23198 (build: Parse version information in msvc-autogen.py by CallMeMisterOwl)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@DrahtBot
Copy link
Contributor

DrahtBot commented Oct 7, 2021

Guix builds

File commit 113b863
(master)
commit 0e33c135245b2fe5021aa1308bb4eafa449ce42e
(master and this pull)
SHA256SUMS.part cef142e76613e81b... b465bfc75b4a9995...
*-aarch64-linux-gnu-debug.tar.gz adc3d28770776aef... e4aa8b7b9018629e...
*-aarch64-linux-gnu.tar.gz 98d5eefedff98988... d8b9feff61fe8a6f...
*-arm-linux-gnueabihf-debug.tar.gz f6039364f26dc133... 2838d9d15a296e78...
*-arm-linux-gnueabihf.tar.gz 0c5c55979203b62a... 9878a882c041e78a...
*-osx-unsigned.dmg ec644de45a41ea59... 99ec2351884ccc8c...
*-osx-unsigned.tar.gz 913228fa5ae5f6cc... 1ef1c8a197131d59...
*-osx64.tar.gz ec065732196d4d84... 88de5a967b42116b...
*-powerpc64-linux-gnu-debug.tar.gz 5ac7cc1a4a23bce9... c3fc2c6cb1900b65...
*-powerpc64-linux-gnu.tar.gz 65acd05354b5d45f... 566a2be08d3f2c98...
*-powerpc64le-linux-gnu-debug.tar.gz 58a17617fc3bce0b... d25cb8f59ebf0889...
*-powerpc64le-linux-gnu.tar.gz 51803987d606fe8f... 2c8bae71615fe5f1...
*-riscv64-linux-gnu-debug.tar.gz 1a6a587ac8d82232... 4cc064e85d0a63b9...
*-riscv64-linux-gnu.tar.gz 2dd1e3a266c00d59... a503e52532528dea...
*-win-unsigned.tar.gz 8766d679ac4fda9e... 58eb31d9ee80cada...
*-win64-debug.zip a0ff6a43a6b029e3... c38e5c18a397c271...
*-win64-setup-unsigned.exe 59c2e6aa554a9ce5... 4648a73dec86f38c...
*-win64.zip d445bf1337e6ca11... 6d61afabff9df58e...
*-x86_64-linux-gnu-debug.tar.gz 8463e1b9f1a8bba6... 4454c0494f2c0461...
*-x86_64-linux-gnu.tar.gz e9533a964c50a19f... 1619b584f5e16603...
*.tar.gz d20968f10addb58f... 475d0a6c40e201d4...
guix_build.log db438b6542214e61... 3ecf48cedfeb5da0...
guix_build.log.diff c2459d1cddd40f25...

@practicalswift
Copy link
Contributor

practicalswift commented Oct 7, 2021

Some additional review comments:

I've done some additional digging and Linux 4.4 (2016) is the oldest kernel that is not EOL.

Thus it should only be the syscalls introduced after Linux 4.4 that are part of the LINUX_SYSCALLS map that might cause compile-time errors due to __NR_<syscall> being undefined (on supported non-EOL systems).

This should be the complete list of such syscalls:

  • Linux 4.5 (2016): copy_file_range
  • Linux 4.6 (2016): preadv2, pwritev2
  • Linux 4.8 (2016): pkey_alloc, pkey_free, pkey_mprotect
  • Linux 4.11 (2017): statx
  • Newer syscalls are not included as part of LINUX_SYSCALLS. Note that LINUX_SYSCALLS is only used for friendly printing of syscall names, and thus it doesn't need to be complete with the most recently introduced syscalls. If a syscall is not present in LINUX_SYSCALLS the syscall number will be printed in error messages, but not the syscall name.

All the syscalls listed above are covered (either via #define or #ifdef) after the merge of this PR.

Thus we shouldn't see any more compile-time issues on supported non-EOL systems after the merge of this PR :)

@laanwj
Copy link
Member Author

laanwj commented Oct 7, 2021

@practicalswift Whoa, thanks for doing some software archeology there.

Thus we shouldn't see any more compile-time issues on supported non-EOL systems after the merge of this PR :)

I think the main users of EOL kernels are people stuck with the vendor kernel for some embedded board. But even if ARM would be supported for sandboxing, it's always possible to disable it with configure, it's not like this prevents compilation altogether.

@laanwj laanwj merged commit 6334ff7 into bitcoin:master Oct 7, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Oct 7, 2021
…l 4.4.0

ac402e7 util: Conditionalize some syscalls in syscall name table (W. J. van der Laan)
64085b3 util: Add __NR_copy_file_range syscall constant for sandbox (W. J. van der Laan)

Pull request description:

  Make the new syscall sandbox compilable with kernel 4.4.0.
  This defines a further syscall constant `__NR_copy_file_range` to make sure all syscalls used in the profile are available even if not defined in the kernel headers.

  Also, make a few syscalls optional in the syscall name table:

  - `__NR_pkey_alloc`
  - `__NR_pkey_free`
  - `__NR_pkey_mprotect`
  - `__NR_preadv2`
  - `__NR_pwritev2`

ACKs for top commit:
  practicalswift:
    cr ACK ac402e7

Tree-SHA512: be6c55bf0a686bcdfad0b80b950d0d7d77a559ac234fc997b47514bdba44865a371c96dd8d34a811ba46424a84f410e23f75485b9b1e69e529b7d40e0b4b91b8
@bitcoin bitcoin locked and limited conversation to collaborators Oct 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants