Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: expect mmap to fail in seccomp test on s390 and s390x #14162

Closed

Conversation

rbalint
Copy link
Contributor

@rbalint rbalint commented Nov 27, 2019

No description provided.

@yuwata yuwata added the tests label Nov 27, 2019
@rbalint
Copy link
Contributor Author

rbalint commented Nov 27, 2019

The patch is carried in Ubuntu starting with ubuntu/243-2ubuntu1 (in 20.04). Since CI is run on bionic here, this will make the s390x run fail.
I'd like to run Ubuntu CI tests on latest Ubuntu release instead of the latest LTS to make the CI environment more similar to where new systemd releases would eventually land and possibly map stable branch CI to the appropriate older Ubuntu releases, but the infrastructure is not there yet.
If we want to keep running CI on latest Ubuntu LTS then we can wait with merging this fix until 20.04 is released.

@poettering poettering added the good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed label Nov 27, 2019
@rbalint
Copy link
Contributor Author

rbalint commented Nov 27, 2019

There are a few more fixes at https://github.com/cpaelzer/systemd/commits/fix-MemoryDenyWriteExecute-x86-s390-bug-1853852-UPSTREAM to work with libseccomp 2.4.2. Those may make the change pass CI even on 18.04.

@keszybz keszybz added ci-fails/needs-rework 🔥 Please rework this, the CI noticed an issue with the PR and removed good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed labels Nov 27, 2019
@keszybz
Copy link
Member

keszybz commented Nov 27, 2019

bionic-s390x dies with

/* test_memory_deny_write_execute_mmap */
Operating on architecture: s390
Failed to add pkey_mprotect() rule for architecture s390, skipping: Numerical argument out of domain
Operating on architecture: s390x
Failed to add pkey_mprotect() rule for architecture s390x, skipping: Numerical argument out of domain
Assertion 'p == MAP_FAILED' failed at src/test/test-seccomp.c:536, 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:550, function test_memory_deny_write_execute_mmap(). Aborting.
--- test-seccomp end ---

@keszybz keszybz added postponed and removed ci-fails/needs-rework 🔥 Please rework this, the CI noticed an issue with the PR labels Nov 27, 2019
@keszybz
Copy link
Member

keszybz commented Nov 27, 2019

Ah, the failure is expected. Dunno, can we make it pass on both? We certainly can't merge this until the tests pass, and having tests that only pass on some versions of the testbed is problematic.

@poettering
Copy link
Member

hmm, so what's the background here? why does this work on some s390 and not on others?

@cpaelzer
Copy link
Contributor

cpaelzer commented Dec 4, 2019

From all my tests I think this "just" depends on the version of kernel and libseccomp.
Rbalint has this code for 20.04 and there it works, but the CI runs on Bionic which makes this fix appear as failing.

This is the mmap test, for the shmat test in the past it was already given up to track which combination works well and which doesn't. See commit 67fb5f3 for that.

This also affects my PR #14167 which I formerly based on this one, maybe we can add a similar strategy as we did for shmat to the mmap test.
I'm making mmap do the same in my PR #14167 which should fix this CI issue here (it also would supersede/replace this PR as s390/s390x would no more need to be added to the ifdef check line).

@yuwata yuwata removed the postponed label Dec 4, 2019
@cpaelzer
Copy link
Contributor

cpaelzer commented Dec 4, 2019

FYI - Tests on #14167 now succeeded on s390x on the 18.04 CI as well as in Ubuntu 20.04 locally. When #14167 is merged we can close/abort this PR here IMHO.

@poettering
Copy link
Member

OK, let's close in favour of #14167

@poettering poettering closed this Dec 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

5 participants