systemd 237-3ubuntu10.14 ADT test failure on Bionic ppc64el (test-seccomp)

This is not a bug in systemd.

The problem is that libseccomp was patched at version 2.3.1-2.1ubuntu4.1, in d/p/lp-1815415-arch-update-syscalls-for-Linux-4.9.patch, to add a definition for the pkey_mprotect syscall. The systemd test case checks for the definition of this syscall, and uses it if it's defined, which with the new libseccomp library, it is.

However, the ppc64el kernel doesn't add support for the pkey_mprotect syscall until 4.16.

Either the bionic 4.15 kernel should add support for pkey_mprotect syscall (commit 3350eb2ea127978319ced883523d828046af4045, at least, possibly more), or libseccomp should remove it, at least for ppc64el builds on bionic. Adding support for it to the kernel for ppc64el is probably the best approach.

i marked this as against both libseccomp and the kernel, but as I mentioned in the last comment, only 1 of them needs fixing. My suggestion for the best approach is to add pkey_mprotect support to the bionic ppc64el kernel.

coverletter explanation as sent to kernel-team list:

In bionic, all archs provided by Ubuntu either define __NR_pkey_mprotect
(arm/x86) or define __IGNORE_pkey_mprotect (powerpc/s390). This value was
used, until libseccomp was updated via bug 1815415, to instead (if
__NR_pkey_mprotect was not defined by the kernel headers) define it as a
negative error value:

+#define __PNR_pkey_mprotect -10201
+#ifndef __NR_pkey_mprotect
+#define __NR_pkey_mprotect __PNR_pkey_mprotect
+#endif /* __NR_pkey_mprotect */

systemd, the next time it was built against libseccomp, pulled that ...

> After these patches are applied to bionic, both libseccomp and systemd
> will need to be rebuilt - libseccomp rebuilt against the kernel
> headers, and systemd against the libseccomp headers.

technically both will need to be rebuilt against the new kernel headers, but systemd doesn't need to wait for libseccomp to be rebuilt, as it should get the correct value as soon as the kernel headers are updated, since the libseccomp header file only defines __NR_pkey_mprotect (as an invalid value) if the kernel header doesn't define it.

I'm leaving this as marked against libseccomp, as I don't understand why it is redefining kernel header values as invalid numbers in its public header file (so that other programs using its header files pull in those invalid syscall numbers). I don't think it should but I'll leave the decision to discuss this upstream to someone more familiar with libseccomp.

FYI, please note that seccomp 2.4.1 was pushed to bionic in https://usn.ubuntu.com/4001-1/ on 2019/05/30. It shouldn't affect this bug report AFAICT because while the 2.4.1 Ubuntu packaging drops these patches, the upstream commits for lp-1815415-arch-update-syscalls-for-Linux-4.9.patch and lp-1815415-update-the-syscall-tables-to-4.10.patch are both included in 2.4.1. Based on the 2.4.1 changelog, nothing else was changed in this area, so 2.4.1 should be affected in the same way as 2.3.1-2.1ubuntu4.1.

This bug is awaiting verification that the kernel in -proposed solves the problem. Please test the kernel and update this bug with the results. If the problem is solved, change the tag 'verification-needed-bionic' to 'verification-done-bionic'. If the problem still exists, change the tag 'verification-needed-bionic' to 'verification-failed-bionic'.

If verification is not done by 5 working days from today, this fix will be dropped from the source code, and this bug will be closed.

See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you!

I rebuilt systemd using the 4.15.0-55 kernel headers:
https://launchpad.net/~ddstreet/+archive/ubuntu/lp1821625.2

and the test now passes:
https://objectstorage.prodstack4-5.canonical.com/v1/AUTH_77e2ada1e7a84929a74ba3b87153c0ac/autopkgtest-bionic-ddstreet-lp1821625.2/bionic/ppc64el/s/systemd/20190704_021031_ebc9b@/log.gz

systemd will require a rebuild to pick up this fix in the kernel headers, but that happens often enough I don't think a no-change rebuild is needed.

Also, these packages reverse-dep on the libseccomp-dev package (that provides the faulty __NR_pkey_mprotect define) so they might need rebuilding as well, if they use that define for anything:

$ reverse-depends -r bionic -b libseccomp-dev
Reverse-Build-Depends
=====================
* apt
* chrony
* docker.io
* flatpak
* gnome-desktop3
* golang-github-seccomp-libseccomp-golang
* kscreenlocker
* lxc
* man-db
* ocserv
* qemu
* snapd
* stenographer
* systemd
* tor
* tracker-miners
* usbguard

Per [1] this symbol is present in: firejail, elogind, valgrind, glibc, musl, linux, systemd, strace, stress-ng, libseccomp, qemu

At least for qemu I checked and those are only references in the embedded linux headers. No rebuild needed for that, most of the others seem safe as well - maybe someone to look into stress-ng and strace. The one more improtant case might be libseccomp and as already identified systemd.

We had a recent upload of a newer seccomp [2] that was still built against 4.15.0-47.50.
@Dan did that already contain the fix you expect or would considering a seccomp be useful?

[1]: https://codesearch.debian.net/search?q=__NR_pkey_mprotect
[2]: https://launchpad.net/~ubuntu-security/+archive/ubuntu/ppa/+build/16742797

> @Dan did that already contain the fix you expect or would considering a seccomp be useful?

For libseccomp, it looks like it doesn't need a rebuild; it appears to only reference the symbol in its header (which is what causes this problem for other packages that pull in that header); in its code, the only reference is where its syscall number is defined is inside its ppc64_syscall_table (and ppc_syscall_table), where it is defined correctly.

ddstreet@thorin:~/bugs/lp1821625/sym-check/libseccomp/libseccomp-2.4.1$ grep -r pkey_mprotect
include/seccomp.h.in:#define __PNR_pkey_mprotect -10201
include/seccomp.h.in:#ifndef __NR_pkey_mprotect
include/seccomp.h.in:#define __NR_pkey_mprotect __PNR_pkey_mprotect
include/seccomp.h.in:#endif /* __NR_pkey_mprotect */
include/seccomp.h:#define __PNR_pkey_mprotect -10201
include/seccomp.h:#ifndef __NR_pkey_mprotect
include/seccomp.h:#define __NR_pkey_mprotect __PNR_pkey_mprotect
include/seccomp.h:#endif /* __NR_pkey_mprotect */
src/arch-ppc-syscalls.c: { "pkey_mprotect", 386 },
src/arch-x32-syscalls.c: { "pkey_mprotect", (X32_SYSCALL_BIT + 329) },
src/arch-mips-syscalls.c: { "pkey_mprotect", (__SCMP_NR_BASE + 363) },
src/arch-x86-syscalls.c: { "pkey_mprotect", 380 },
src/arch-s390-syscalls.c: { "pkey_mprotect", __PNR_pkey_mprotect },
src/arch-parisc-syscalls.c: { "pkey_mprotect", __PNR_pkey_mprotect },
src/arch-s390x-syscalls.c: { "pkey_mprotect", __PNR_pkey_mprotect },
src/arch-ppc64-syscalls.c: { "pkey_mprotect", 386 },
src/arch-aarch64-syscalls.c: { "pkey_mprotect", 288 },
src/arch-arm-syscalls.c: { "pkey_mprotect", (__SCMP_NR_BASE + 394) },
src/arch-mips64-syscalls.c: { "pkey_mprotect", (__SCMP_NR_BASE + 323) },
src/arch-x86_64-syscalls.c: { "pkey_mprotect", 329 },
src/arch-mips64n32-syscalls.c: { "pkey_mprotect", (__SCMP_NR_BASE + 327) },

I still don't understand why libseccomp ever decided to define __NR_pkey_mprotect to a negative number (if the kernel header doesn't define it) in its public header file. IMHO that should be done in its code, or at least in a private header file.

Marking this bug as Invalid for libseccomp; I still disagree with their upstream decision re: their header file, but it doesn't appear any rebuild or SRU code change is needed for it for this bug.

> Per [1] this symbol is present in: firejail, elogind, valgrind, glibc, musl, linux,
> systemd, strace, stress-ng, libseccomp, qemu

tl;dr: it looks like glibc and stress-ng are the only packages from that list that might need a rebuild, but as with systemd, neither seem important enough to warrant a special no-change rebuild. They will pick up the new syscall value whenever they are next uploaded for any other bugfix (after the current bionic -proposed kernel is promoted to -updates).

details:

elogind: not included in bionic

firejail: it does use SYS_pkey_mprotect (which is defined as __NR_pkey_mprotect):
ddstreet@thorin:~/bugs/lp1821625/sym-check/firejail/firejail-0.9.52$ grep -r pkey_mprotect
src/man/firejail.txt:the arguments of mmap, mmap2, mprotect, pkey_mprotect and shmat system
src/fseccomp/seccomp.c: // same for pkey_mprotect(,,PROT_EXEC), where available
src/fseccomp/seccomp.c:#ifdef SYS_pkey_mprotect
src/fseccomp/seccomp.c: BPF_JUMP(BPF_JMP+BPF_JEQ+BPF_K, SYS_pkey_mprotect, 0, 5),

however, the latest build available:
https://launchpad.net/ubuntu/+source/firejail/0.9.52-2
for ppc64el:
https://launchpad.net/ubuntu/+source/firejail/0.9.52-2/+build/13948498
was built with libseccomp2, not libseccomp-dev (which provides the problematic public header file):
Get:57 http://ftpmaster.internal/ubuntu bionic/main ppc64el libseccomp2 ppc64el 2.3.1-2.1ubuntu3 [47.2 kB]

additionally, it was built with libseccomp2 version 2.3.1-2.1ubuntu3, which was earlier than when the problematic patch was added (per this bug comment 1). So no need to recompile.

musl: appears to only define __NR_pkey_mprotect in its embedded headers, and doesn't include ppc; no need to require recompile:

ddstreet@thorin:~/bugs/lp1821625/sym-check/musl/musl-1.1.19$ grep -r pkey_mprotect
arch/mipsn32/bits/syscall.h.in:#define __NR_pkey_mprotect 6327
arch/mips64/bits/syscall.h.in:#define __NR_pkey_mprotect 5323
arch/arm/bits/syscall.h.in:#define __NR_pkey_mprotect 394
arch/aarch64/bits/syscall.h.in:#define __NR_pkey_mprotect 288
arch/or1k/bits/syscall.h.in:#define __NR_pkey_mprotect 288
arch/x32/bits/syscall.h.in:#define __NR_pkey_mprotect (0x40000000 + 329)
arch/i386/bits/syscall.h.in:#define __NR_pkey_mprotect 380
arch/x86_64/bits/syscall.h.in:#define __NR_pkey_mprotect 329
arch/mips/bits/syscall.h.in:#define __NR_pkey_mprotect 4363
arch/microblaze/bits/syscall.h.in:#define __NR_pkey_mprotect 395

qemu: as @cpaelzer said, just in its embedded headers; no need to recompile.

strace: does appear to use pkey_mprotect, but the latest ppc64el build:
https://launchpad.net/ubuntu/+source/strace/4.21-1ubuntu1/+build/14750033
did not build with libseccomp-dev (or libseccomp2) at all. Additionally, it was built 2018-04-11, while the problematic patch was added to libseccomp 2019-02-12. No need to rebuild.

stress-ng: it does include the number in its skip_syscalls list, of syscalls to skip while running the 'enosys' stressor:
ddstreet@thorin:~/bugs/lp1821625/sym-check/stress-ng/stress-ng-0.09.25$ grep -r pkey_mprotect
stress-enosys.c:#if defined(SYS_pkey_mprotect)
stress-enosys.c: SYS_pkey_mprotect,
stress-enosys.c:#if defined(__NR_pkey_mprotect)
stres...

$ reverse-depends -l -r bionic -b libseccomp-dev |& xargs
apt chrony docker.io flatpak gnome-desktop3
golang-github-seccomp-libseccomp-golang kscreenlocker lxc man-db ocserv
qemu snapd stenographer systemd tor tracker-miners usbguard

none of those packages reference pkey_mprotect except systemd, qemu, and snapd; systemd and qemu are discussed above. The snapd package simply lists pkey_mprotect in a list of syscalls and doesn't appear to compile-in the syscall number, and so should not be affected:
ddstreet@thorin:~/bugs/lp1821625/sym-check/snapd/snapd-2.39.2+18.04$ grep -r pkey_mprotect
cmd/snap-seccomp/syscalls/syscalls.go: "pkey_mprotect",

additionally, snapd is updated quite often, so even if it is affected there should be no need for a no-change rebuild.

I agree with the analysis in comment #13 about stress-ng.

This bug was fixed in the package linux - 4.15.0-55.60

---------------
linux (4.15.0-55.60) bionic; urgency=medium

  * linux: 4.15.0-55.60 -proposed tracker (LP: #1834954)

  * Request backport of ceph commits into bionic (LP: #1834235)
    - ceph: use atomic_t for ceph_inode_info::i_shared_gen
    - ceph: define argument structure for handle_cap_grant
    - ceph: flush pending works before shutdown super
    - ceph: send cap releases more aggressively
    - ceph: single workqueue for inode related works
    - ceph: avoid dereferencing invalid pointer during cached readdir
    - ceph: quota: add initial infrastructure to support cephfs quotas
    - ceph: quota: support for ceph.quota.max_files
    - ceph: quota: don't allow cross-quota renames
    - ceph: fix root quota realm check
    - ceph: quota: support for ceph.quota.max_bytes
    - ceph: quota: update MDS when max_bytes is approaching
    - ceph: quota: add counter for snaprealms with quota
    - ceph: avoid iput_final() while holding mutex or in dispatch thread

  * QCA9377 isn't being recognized sometimes (LP: #1757218)
    - SAUCE: USB: Disable USB2 LPM at shutdown

  * hns: fix ICMP6 neighbor solicitation messages discard problem (LP: #1833140)
    - net: hns: fix ICMP6 neighbor solicitation messages discard problem
    - net: hns: fix unsigned comparison to less than zero

  * Fix occasional boot time crash in hns driver (LP: #1833138)
    - net: hns: Fix probabilistic memory overwrite when HNS driver initialized

  * use-after-free in hns_nic_net_xmit_hw (LP: #1833136)
    - net: hns: fix KASAN: use-after-free in hns_nic_net_xmit_hw()

  * hns: attempt to restart autoneg when disabled should report error
    (LP: #1833147)
    - net: hns: Restart autoneg need return failed when autoneg off

  * systemd 237-3ubuntu10.14 ADT test failure on Bionic ppc64el (test-seccomp)
    (LP: #1821625)
    - powerpc: sys_pkey_alloc() and sys_pkey_free() system calls
    - powerpc: sys_pkey_mprotect() system call

  * [UBUNTU] pkey: Indicate old mkvp only if old and curr. mkvp are different
    (LP: #1832625)
    - pkey: Indicate old mkvp only if old and current mkvp are different

  * [UBUNTU] kernel: Fix gcm-aes-s390 wrong scatter-gather list processing
    (LP: #1832623)
    - s390/crypto: fix gcm-aes-s390 selftest failures

  * System crashes on hot adding a core with drmgr command (4.15.0-48-generic)
    (LP: #1833716)
    - powerpc/numa: improve control of topology updates
    - powerpc/numa: document topology_updates_enabled, disable by default

  * Kernel modules generated incorrectly when system is localized to a non-
    English language (LP: #1828084)
    - scripts: override locale from environment when running recordmcount.pl

  * [UBUNTU] kernel: Fix wrong dispatching for control domain CPRBs
    (LP: #1832624)
    - s390/zcrypt: Fix wrong dispatching for control domain CPRBs

  * CVE-2019-11815
    - net: rds: force to destroy connection if t_sock is NULL in
      rds_tcp_kill_sock().

  * Sound device not detected after resume from hibernate (LP: #1826868)
    - drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled
    - drm/i915: Save the old CDCLK atomic state
...

to close this out, the latest systemd build, version 237-3ubuntu10.25, picked up the correct pkey_mprotect value from the updated kernel and now does not fail the test.

This bug is awaiting verification that the kernel in -proposed solves the problem. Please test the kernel and update this bug with the results. If the problem is solved, change the tag 'verification-needed-xenial' to 'verification-done-xenial'. If the problem still exists, change the tag 'verification-needed-xenial' to 'verification-failed-xenial'.

If verification is not done by 5 working days from today, this fix will be dropped from the source code, and this bug will be closed.

See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you!

$ reverse-depends -b -r xenial libseccomp-dev
Reverse-Build-Depends
=====================
* golang-github-seccomp-libseccomp-golang
* lxc
* ocserv
* qemu
* systemd
* tor
* ubuntu-core-launcher

none of these appear to reference pkey_mprotect, so none should need a rebuild with the xenial-hwe kernel; marking verification-xenial-done based on that.