hard to reproduce issues in systemd autopkgtest against new libseccomp 2.4.2

Bug #1853852 reported by Christian Ehrhardt 
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
libseccomp
New
Unknown
libseccomp (Ubuntu)
Invalid
Undecided
Unassigned
systemd (Ubuntu)
Fix Released
Undecided
Unassigned

Bug Description

Hi,
I'm mostly reporting this if to one of the people watching systemd more closely this is in any form a known issue or if there are any hints.

I recently merged libseccomp 2.4.2 and after a few initial cleanups that worked well.
But on propsoed-migration I hit systemd test issues.

I have read about issues with arm NR_open defines - I had the same in chrony - but that is fixed in libseccomp and that isn't failing in systemd.

i386 and s390x (only those) have failing tests
- http://autopkgtest.ubuntu.com/packages/s/systemd/focal/s390x
- http://autopkgtest.ubuntu.com/packages/s/systemd/focal/i386

Example:
https://objectstorage.prodstack4-5.canonical.com/v1/AUTH_77e2ada1e7a84929a74ba3b87153c0ac/autopkgtest-focal/focal/s390x/s/systemd/20191120_105726_aea23@/log.gz

Failnig subtests are:
root-unittests FAIL non-zero exit status 134
upstream FAIL non-zero exit status 1

And looking at the details of root-unittest I found: http://paste.ubuntu.com/p/N7q9PX3hFN/
====== test-seccomp =======
...
/* test_memory_deny_write_execute_mmap */
Operating on architecture: s390
Failed to add shmat() rule for architecture s390, skipping: Invalid argument
Operating on architecture: s390x
Failed to add shmat() rule for architecture s390x, skipping: Invalid argument
Assertion 'p == MAP_FAILED' failed at src/test/test-seccomp.c:493, function test_memory_deny_write_execute_mmap(). Aborting.
memoryseccomp-mmap terminated by signal ABRT.
Assertion 'wait_for_terminate_and_check("memoryseccomp-mmap", pid, WAIT_LOG) == EXIT_SUCCESS' failed at src/test/test-seccomp.c:507, function test_memory_deny_write_execute_mmap(). Aborting.
FAIL: test-seccomp (code: 134)

But when installing source of systemd and the new libseccomp in a Focal VM with proposed enabled it works just fine. Actually I just found that it does have a good RC but breaks so maybe it is debuggable after all.

Related branches

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Seems to be recreatable with
$ sudo /usr/lib/systemd/tests/test-seccomp

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

x86 32bit is rather similar:
/* test_memory_deny_write_execute_mmap */
Operating on architecture: x86
Failed to add shmat() rule for architecture x86, skipping: Invalid argument
Assertion 'p == MAP_FAILED' failed at src/test/test-seccomp.c:493, function test_memory_deny_write_execute_mmap(). Aborting.
memoryseccomp-mmap terminated by signal ABRT.
Assertion 'wait_for_terminate_and_check("memoryseccomp-mmap", pid, WAIT_LOG) == EXIT_SUCCESS' failed at src/test/test-seccomp.c:507, function test_memory_deny_write_execute_mmap(). Aborting.
FAIL: test-seccomp (code: 134)

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

(gdb) bt
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1 0x000003fffdd2b232 in __GI_abort () at abort.c:79
#2 0x000003fffdb03a64 in log_assert_failed_realm () from /lib/systemd/libsystemd-shared-243.so
#3 0x000002aa0000746e in test_memory_deny_write_execute_mmap () at ../src/test/test-seccomp.c:507
#4 0x000002aa00002a48 in main (argc=<optimized out>, argv=<optimized out>) at ../src/test/test-seccomp.c:987

The test is:
static void test_memory_deny_write_execute_mmap(void) {
...
        if (pid == 0) {
                void *p;

                p = mmap(NULL, page_size(), PROT_WRITE|PROT_EXEC, MAP_PRIVATE|MAP_ANONYMOUS, -1,0);
                assert_se(p != MAP_FAILED);
                assert_se(munmap(p, page_size()) >= 0);

                p = mmap(NULL, page_size(), PROT_WRITE|PROT_READ, MAP_PRIVATE|MAP_ANONYMOUS, -1,0);
                assert_se(p != MAP_FAILED);
                assert_se(munmap(p, page_size()) >= 0);

                assert_se(seccomp_memory_deny_write_execute() >= 0);

                p = mmap(NULL, page_size(), PROT_WRITE|PROT_EXEC, MAP_PRIVATE|MAP_ANONYMOUS, -1,0);
#if defined(__x86_64__) || defined(__i386__) || defined(__powerpc64__) || defined(__arm__) || defined(__aarch64__) || defined(__s390__) || defined(__s390x__)
                assert_se(p == MAP_FAILED);
                assert_se(errno == EPERM);
#else /* unknown architectures */
                assert_se(p != MAP_FAILED);
                assert_se(munmap(p, page_size()) >= 0);
#endif

                p = mmap(NULL, page_size(), PROT_WRITE|PROT_READ, MAP_PRIVATE|MAP_ANONYMOUS, -1,0);
                assert_se(p != MAP_FAILED);
                assert_se(munmap(p, page_size()) >= 0);

                _exit(EXIT_SUCCESS);
        }

        assert_se(wait_for_terminate_and_check("memoryseccomp-mmap", pid, WAIT_LOG) == EXIT_SUCCESS);

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

in GDB with follow-fork-mode child I can check which call is actually failing in the child:

It is this one:
p = mmap(NULL, page_size(), PROT_WRITE|PROT_EXEC, MAP_PRIVATE|MAP_ANONYMOUS, -1,0);
assert_se(p == MAP_FAILED);

It expects a fail (due to seccomp block) but does not get that.

Take it with a grain of salt, that is the packaged binary with -Ox optimizations.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Uh there is something related as Ubuntu Delta:

From: Balint Reczey <email address hidden>
Date: Tue, 22 Oct 2019 17:10:17 +0200
Subject: test: expect mmap to fail in seccomp test on s390 and s390x

(cherry picked from commit a81f7aad9a5ddeebbce002e2da36e1dd84f51b36)
---
 src/test/test-seccomp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/test/test-seccomp.c b/src/test/test-seccomp.c
index a906070..9881768 100644
--- a/src/test/test-seccomp.c
+++ b/src/test/test-seccomp.c
@@ -489,7 +489,7 @@ static void test_memory_deny_write_execute_mmap(void) {
                 assert_se(seccomp_memory_deny_write_execute() >= 0);

                 p = mmap(NULL, page_size(), PROT_WRITE|PROT_EXEC, MAP_PRIVATE|MAP_ANONYMOUS, -1,0);
-#if defined(__x86_64__) || defined(__i386__) || defined(__powerpc64__) || defined(__arm__) || defined(__aarch64__)
+#if defined(__x86_64__) || defined(__i386__) || defined(__powerpc64__) || defined(__arm__) || defined(__aarch64__) || defined(__s390__) || defined(__s390x__)
                 assert_se(p == MAP_FAILED);
                 assert_se(errno == EPERM);
 #else /* unknown architectures */

Maybe that isn't true anymore?
In any case it could explain why upstream might not see/discuss about it.
But then OTOH remember that 32bit seems to be affected the same way

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Adding [1] might be worth in general - this is the patch for the arm related discussion which seems applied upstream.

[1]: https://github.com/systemd/systemd/commit/4df8fe8415eaf4abd5b93c3447452547c6ea9e5f

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

I'm currently checking if I can build locally for quick turnaround times of tests or if I need full LP builds every time ...

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Ok, confirmed the patch debian/patches/test-expect-mmap-to-fail-in-seccomp-test-on-s390-and-s390.patch has to be taken out to make it work on s390x.

And most likely the same needs to happen for x86-32bit - there we don't have a patch.
But this most likely also needs to be switched to no more work to block due to changes in libseccomp. That change then could be discussed upstream to identify the reasons in libseccomp causing this.

Maybe until then rbalint can report why this patch was added and if there was a discussion associated.

tags: added: update-excuse
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

seccomp_memory_deny_write_execute [1] in src/shared/seccomp-util.c is the one setting the seccomp rules called from test test_memory_deny_write_execute_mmap

It fails at: test_memory_deny_write_execute_mmap
But later on we usually get debug info about used syscalls at: test_memory_deny_write_execute_shmat [2]

Lets compare this debug info in good/bad case just exchaning libseccomp 2.4.1 <-> 2.4.2

The test aborts at the first error, so we need to "mask" the test_memory_deny_write_execute_mmap to get that data. Also with the "fix" for the new seccomp obviously the old one is failing.

Note that at test_memory_deny_write_execute_shmat the arch list expected to work is just:
#if defined(__x86_64__) || defined(__arm__) || defined(__aarch64__)

While at test_memory_deny_write_execute_mmap it was
#if defined(__x86_64__) || defined(__i386__) || defined(__powerpc64__) || defined(__arm__) || defined(__aarch64__) || defined(__s390__) || defined(__s390x__)

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

I'll stay at 32bit intel for now as it is easier to get to for most developers compared to s390x

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

We know it fails at shmat, so we most likely can focus on that one.

i386 2.4.2
/* test_memory_deny_write_execute_shmat */
arch x86: SCMP_SYS(mmap) = 90
arch x86: SCMP_SYS(mmap2) = 192
arch x86: SCMP_SYS(shmget) = 395
arch x86: SCMP_SYS(shmat) = 397
arch x86: SCMP_SYS(shmdt) = 398
Operating on architecture: x86
Failed to add shmat() rule for architecture x86, skipping: Invalid argument
shmat(SHM_EXEC): Success
shmat(0): Success
memoryseccomp-shmat succeeded.

/* test_memory_deny_write_execute_shmat */
arch x86: SCMP_SYS(mmap) = 90
arch x86: SCMP_SYS(mmap2) = 192
arch x86: SCMP_SYS(shmget) = 395
arch x86: SCMP_SYS(shmat) = 397
arch x86: SCMP_SYS(shmdt) = 398
Operating on architecture: x86
shmat(SHM_EXEC): Success
shmat(0): Success

So the IDs detected did not change, but the behavior did.

libseccomp saw the bump to kernel 5.4 to include
- { "shmat", __PNR_shmat },
+ { "shmat", 397 },
But as we saw the effective ID didn't change.

[1]: https://github.com/systemd/systemd/blob/master/src/shared/seccomp-util.c#L1584
[2]: https://github.com/systemd/systemd/blob/master/src/test/test-seccomp.c#L560

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

I've simplified the test to a small case - running that I can reproduce the error.
This should be enough to go to upstreams with it.

cat > test-seccomp-shmat.c << EOF
#include <linux/seccomp.h>

#include <errno.h>
#include <seccomp.h>
#include <stdio.h>

#include <sys/shm.h>

/*
 * Test issues with libseccomp 2.4.1 -> 2.4.2
 * Derived from systemd testcase test_memory_deny_write_execute_shmat
 * which fails to install shmat rules with 2.4.2 on i386 and s390x
 */

int main()
{
   int shmat_syscall = -1;
   int rc = -1;
   scmp_filter_ctx ctx;

   ctx = seccomp_init(SCMP_ACT_ALLOW);
   if (ctx == NULL)
       return -1;

   shmat_syscall = SCMP_SYS(shmat);
   printf("SCMP_SYS(shmat) = %d\n", shmat_syscall);

   rc = seccomp_rule_add_exact(ctx, SCMP_ACT_ERRNO(EPERM), shmat_syscall, 1, SCMP_A2(SCMP_CMP_MASKED_EQ, SHM_EXEC, SHM_EXEC));
   printf("Rule installed RC = %d\n", rc);

   return 0;
}
EOF

$ gcc -Wall test-seccomp-shmat.c -o test-seccomp-shmat -lseccomp

i386:
2.4.1:
./test-seccomp-shmat
SCMP_SYS(shmat) = 397
Rule installed RC = 0
2.4.2
./test-seccomp-shmat
SCMP_SYS(shmat) = 397
Rule installed RC = -22

s390x looks identical to the i386 output

Note: rebuilding on new libseccomp2 does not change this behavior

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Now that I could properly describe the issue I have filed upstream bug https://github.com/seccomp/libseccomp/issues/193

Changed in libseccomp:
status: Unknown → New
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Discussions with libseccomp upstream go well.
I might eventually need to go to upstream systemd and propose a fix, but before doing so I need more clarification with libseccomp upstream and some more testing.

Experimental branch for packaging experiments (d/p/patch file):
=> https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/log/?h=fix-MemoryDenyWriteExecute-x86-s390-bug-1853852

Experimental branch to be submitted upstream (individual patches) after being polished and tested:
=> https://github.com/cpaelzer/systemd/tree/fix-MemoryDenyWriteExecute-x86-s390-bug-1853852-UPSTREAM

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Now also systemd discussion and code started in https://github.com/systemd/systemd/pull/14167

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

@rbalint how would you feel about - for now - reverting debian/patches/test-expect-mmap-to-fail-in-seccomp-test-on-s390-and-s390.patch and instead also mark __i386__ as failing-to-protect?

That would allow the current combination of systemd&libseccomp in Focal to pass.
Once this issue is sorted out between the upstreams we can adapt as needed.

Well, lets give upstreams some time now - but knowing your opinion on this would be nice.

Furthermore FYI systemd is atm FTFBS on arm64 and needs https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?h=fix-MemoryDenyWriteExecute-x86-s390-bug-1853852&id=90a9551007b37be0c8a2569b918c33f56649682b

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

FYI: Discussions with both upstreams continue, I'm right now trying a new spin with a different approach that does not use the _exact calls and will get back to the upstreams once (if) that works.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Issue is in systemd, fix known (now).
I'll prep an MP to discuss, but given how much extra churn systemd updates usually cause this most likely won't be uploaded "on its own"

Changed in libseccomp (Ubuntu):
status: New → Invalid
Changed in systemd (Ubuntu):
status: New → Triaged
Revision history for this message
Launchpad Janitor (janitor) wrote :
Download full text (16.7 KiB)

This bug was fixed in the package systemd - 244-3ubuntu1

---------------
systemd (244-3ubuntu1) focal; urgency=medium

  [ Balint Reczey ]
  * Merge to Ubuntu from Debian unstable
  * Refresh patches:
    - Dropped changes:
      * d/t/control: mark udev test skippable.
        File: debian/tests/control
        https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?id=c3419bd2a30a78d05cca9c38e50c9726de7e7632
      * test-execute: Filter /dev/.lxc in exec-dynamicuser-statedir.service.
        File: debian/patches/test-execute-Filter-dev-.lxc-in-exec-dynamicuser-statedir.patch
        https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?id=75af888d5552f706b86182a56f12ccc8e83ca04e
      * Pass personality test even when i386 userland runs on amd64 kernel
        File: debian/patches/debian/UBUNTU-test-Pass-personality-test-even-when-i386-userland-runs-o.patch
        https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?id=42e0bfc426f19430f6768ef4922a9531a345765f
      * Fix resolved fallback to TCP (LP: #1849658)
        Author: Dan Streetman
        File: debian/patches/resolved-set-stream-type-during-DnsStream-creation.patch
        https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?id=f1ee30b13c9d2d34968b09ce620f3bc24a1a78c7
    - Remaining changes:
      * Recommend networkd-dispatcher
        File: debian/control
        https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?id=d1e3b2c7e4757119da0d550b0b3c0a6626a176dc
      * Enable EFI/bootctl on armhf.
        File: debian/control
        https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?id=043122f7d8a1487bfd357e815a6ece1ceea6e7d1
      * debian/control: strengthen dependencies.
        File: debian/control
        https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?id=d1ecf0c372f5212129c85ae60fddf26b2271a1fe
      * Add conflicts with upstart and systemd-shim
        File: debian/control
        https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?id=83ed7496afc7c27be026014d109855f7d0ad1176
      * Specify Ubuntu's Vcs-Git
        File: debian/control
        https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?id=fd832930ef280c9a4a9dda2440d5a46a6fdb6232
      * Ubuntu/extra: ship dhclient-enter hook.
        Files:
        - debian/extra/dhclient-enter-resolved-hook
        - debian/rules
        https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?id=f3398a213f80b02bf3db0c1ce9e22d69f6d56764
        https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?id=258893bae8cbb12670e4807636fe8f7e9fb5407a
        https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?id=0725c1169ddde4f41cacba7af3e546704e2206be
      * udev-udeb: ship modprobe.d snippet to force scsi_mod.scan=sync in d-i.
        Files:
        - debian/extra/modprobe.d-udeb/scsi-mod-scan-sync.conf
        - debian/udev-udeb.install
        https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?id=eb6d8a2b9504917abb7aa2c4035fdbb7b98227f7
      * debian/extra/start...

Changed in systemd (Ubuntu):
status: Triaged → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.