From bed4668d1daeb640c1d55e79e6a1725c81118e39 Mon Sep 17 00:00:00 2001 From: Christian Ehrhardt Date: Wed, 27 Nov 2019 09:52:07 +0100 Subject: [PATCH 1/4] seccomp: fix multiplexed system calls Since libseccomp 2.4.2 more architectures have shmat handled as multiplexed call. Those will fail to be added due to seccomp_rule_add_exact failing on them since they'd need to add multiple rules [1]. See the discussion at https://github.com/seccomp/libseccomp/issues/193 After discussions about the options rejected [2][3] the initial thought of a fallback to the non '_exact' version of the seccomp rule adding the next option is to handle those now affected (i386, s390, s390x) the same way as ppc which ignores and does not block shmat. [1]: https://github.com/seccomp/libseccomp/issues/193 [2]: https://github.com/systemd/systemd/pull/14167#issuecomment-559136906 [3]: https://github.com/systemd/systemd/commit/469830d1 --- src/shared/seccomp-util.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/shared/seccomp-util.c b/src/shared/seccomp-util.c index bd4f004cc8..fc813dd515 100644 --- a/src/shared/seccomp-util.c +++ b/src/shared/seccomp-util.c @@ -1593,22 +1593,23 @@ int seccomp_memory_deny_write_execute(void) { switch (arch) { + /* Note that on some architectures shmat() isn't available, and the call is multiplexed through ipc(). + * We ignore that here, which means there's still a way to get writable/executable + * memory, if an IPC key is mapped like this. That's a pity, but no total loss. */ + case SCMP_ARCH_X86: case SCMP_ARCH_S390: filter_syscall = SCMP_SYS(mmap2); block_syscall = SCMP_SYS(mmap); - shmat_syscall = SCMP_SYS(shmat); + /* shmat multiplexed, see above */ break; case SCMP_ARCH_PPC: case SCMP_ARCH_PPC64: case SCMP_ARCH_PPC64LE: + case SCMP_ARCH_S390X: filter_syscall = SCMP_SYS(mmap); - - /* Note that shmat() isn't available, and the call is multiplexed through ipc(). - * We ignore that here, which means there's still a way to get writable/executable - * memory, if an IPC key is mapped like this. That's a pity, but no total loss. */ - + /* shmat multiplexed, see above */ break; case SCMP_ARCH_ARM: @@ -1619,8 +1620,7 @@ int seccomp_memory_deny_write_execute(void) { case SCMP_ARCH_X86_64: case SCMP_ARCH_X32: case SCMP_ARCH_AARCH64: - case SCMP_ARCH_S390X: - filter_syscall = SCMP_SYS(mmap); /* amd64, x32, s390x, and arm64 have only mmap */ + filter_syscall = SCMP_SYS(mmap); /* amd64, x32 and arm64 have only mmap */ shmat_syscall = SCMP_SYS(shmat); break; From 903659e7b242c3cc897e32835f1918d380b24e5f Mon Sep 17 00:00:00 2001 From: Christian Ehrhardt Date: Wed, 27 Nov 2019 09:57:55 +0100 Subject: [PATCH 2/4] seccomp: ensure rules are loaded in seccomp_memory_deny_write_execute If seccomp_memory_deny_write_execute was fatally failing to load rules it already returned a bad retval. But if any adding filters failed it skipped the subsequent seccomp_load and always returned an rc of 0 even if no rule was loaded at all. Lets fix this requiring to (non fatally-failing) load at least one rule set. Signed-off-by: Christian Ehrhardt --- src/shared/seccomp-util.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/shared/seccomp-util.c b/src/shared/seccomp-util.c index fc813dd515..cf086d22fb 100644 --- a/src/shared/seccomp-util.c +++ b/src/shared/seccomp-util.c @@ -1584,6 +1584,7 @@ assert_cc(SCMP_SYS(shmdt) > 0); int seccomp_memory_deny_write_execute(void) { uint32_t arch; int r; + int loaded = 0; SECCOMP_FOREACH_LOCAL_ARCH(arch) { _cleanup_(seccomp_releasep) scmp_filter_ctx seccomp = NULL; @@ -1678,9 +1679,13 @@ int seccomp_memory_deny_write_execute(void) { return r; if (r < 0) log_debug_errno(r, "Failed to install MemoryDenyWriteExecute= rule for architecture %s, skipping: %m", seccomp_arch_to_string(arch)); + loaded++; } - return 0; + if (loaded == 0) + log_debug_errno(r, "Failed to install any seccomp rules for MemoryDenyWriteExecute="); + + return loaded; } int seccomp_restrict_archs(Set *archs) { From 5ef3ed97e3c718be790b1f38928ca66cb68250a8 Mon Sep 17 00:00:00 2001 From: Christian Ehrhardt Date: Wed, 27 Nov 2019 10:53:50 +0100 Subject: [PATCH 3/4] seccomp: use per arch shmat_syscall At the beginning of seccomp_memory_deny_write_execute architectures can set individual filter_syscall, block_syscall, shmat_syscall values. The former two are then used in the call to add_seccomp_syscall_filter but shmat_syscall is not. Right now all shmat_syscall values are the same, so the change is a no-op, but if ever an architecture is added/modified this would be a subtle source for a mistake so fix it by using shmat_syscall later. Signed-off-by: Christian Ehrhardt --- src/shared/seccomp-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/seccomp-util.c b/src/shared/seccomp-util.c index cf086d22fb..6d42b2d573 100644 --- a/src/shared/seccomp-util.c +++ b/src/shared/seccomp-util.c @@ -1667,7 +1667,7 @@ int seccomp_memory_deny_write_execute(void) { #endif if (shmat_syscall > 0) { - r = add_seccomp_syscall_filter(seccomp, arch, SCMP_SYS(shmat), + r = add_seccomp_syscall_filter(seccomp, arch, shmat_syscall, 1, SCMP_A2(SCMP_CMP_MASKED_EQ, SHM_EXEC, SHM_EXEC)); if (r < 0) From 49219b5c2a654ee6639887aa21a78b41da0576f1 Mon Sep 17 00:00:00 2001 From: Christian Ehrhardt Date: Wed, 4 Dec 2019 11:44:32 +0100 Subject: [PATCH 4/4] seccomp: mmap test results depend on kernel/libseccomp/glibc Like with shmat already the actual results of the test test_memory_deny_write_execute_mmap depend on kernel/libseccomp/glibc of the platform it is running on. There are known-good platforms, but on the others do not assert success (which implies test has actually failed as no seccomp blocking was achieved), but instead make the check dependent to the success of the mmap call on that platforms. Finally the assert of the munmap on that valid pointer should return ==0, so that is what the check should be for in case of p != MAP_FAILED. Signed-off-by: Christian Ehrhardt --- src/test/test-seccomp.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/test/test-seccomp.c b/src/test/test-seccomp.c index 7323e79f7b..69b1c788aa 100644 --- a/src/test/test-seccomp.c +++ b/src/test/test-seccomp.c @@ -535,10 +535,11 @@ static void test_memory_deny_write_execute_mmap(void) { #if defined(__x86_64__) || defined(__i386__) || defined(__powerpc64__) || defined(__arm__) || defined(__aarch64__) 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 + /* Depending on kernel, libseccomp, and glibc versions, other architectures + * might fail or not. Let's not assert success. */ + if (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);