From 3dc51ab2cf228d40438580b24082dbcf1299050c Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Wed, 7 Feb 2024 19:07:07 +0100 Subject: [PATCH 1/2] process-util: use only the least significant byte from personality() The personality() syscall returns a 32-bit value where the top three bytes are reserved for flags that emulate historical or architectural quirks, and only the least significant byte reflects the actual personality we're interested in (in opinionated_personality()). Use the newly defined mask in the corresponding test as well, otherwise the test fails on some more "exotic" architectures that set some of the "quirk" flags: ~# uname -m armv7l ~# build/test-seccomp ... /* test_lock_personality */ current personality=0x0 safe_personality(PERSONALITY_INVALID)=0x800000 Assertion '(unsigned long) safe_personality(current) == current' failed at src/test/test-seccomp.c:970, function test_lock_personality(). Aborting. lockpersonalityseccomp terminated by signal ABRT. Assertion 'wait_for_terminate_and_check("lockpersonalityseccomp", pid, WAIT_LOG) == EXIT_SUCCESS' failed at src/test/test-seccomp.c:996, function test_lock_personality(). Aborting. Aborted (core dumped) See: personality(2) and comments in sys/personality.h --- src/basic/process-util.c | 2 +- src/basic/process-util.h | 5 +++++ src/test/test-seccomp.c | 26 ++++++++++++-------------- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/basic/process-util.c b/src/basic/process-util.c index a94ff38409..002aea9e8a 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -1303,7 +1303,7 @@ int opinionated_personality(unsigned long *ret) { if (current < 0) return current; - if (((unsigned long) current & 0xffff) == PER_LINUX32) + if (((unsigned long) current & OPINIONATED_PERSONALITY_MASK) == PER_LINUX32) *ret = PER_LINUX32; else *ret = PER_LINUX; diff --git a/src/basic/process-util.h b/src/basic/process-util.h index 0fc31f7086..7b2c6cd679 100644 --- a/src/basic/process-util.h +++ b/src/basic/process-util.h @@ -107,6 +107,11 @@ bool oom_score_adjust_is_valid(int oa); #define PERSONALITY_INVALID 0xffffffffLU #endif +/* The personality() syscall returns a 32-bit value where the top three bytes are reserved for flags that + * emulate historical or architectural quirks, and only the least significant byte reflects the actual + * personality we're interested in. */ +#define OPINIONATED_PERSONALITY_MASK 0xFFUL + unsigned long personality_from_string(const char *p); const char *personality_to_string(unsigned long); diff --git a/src/test/test-seccomp.c b/src/test/test-seccomp.c index 279a155cb0..4d3021cf9b 100644 --- a/src/test/test-seccomp.c +++ b/src/test/test-seccomp.c @@ -937,7 +937,7 @@ TEST(native_syscalls_filtered) { } TEST(lock_personality) { - unsigned long current; + unsigned long current_opinionated; pid_t pid; if (!is_seccomp_available()) { @@ -949,24 +949,21 @@ TEST(lock_personality) { return; } - assert_se(opinionated_personality(¤t) >= 0); - /* On ppc64le sanitizers disable ASLR (i.e. by setting ADDR_NO_RANDOMIZE), - * which opinionated_personality() doesn't return. Let's tweak the current - * personality ourselves in such cases. - * See: https://github.com/llvm/llvm-project/commit/78f7a6eaa601bfdd6ae70ffd3da2254c21ff77f9 - */ - if (FLAGS_SET(safe_personality(PERSONALITY_INVALID), ADDR_NO_RANDOMIZE)) - current |= ADDR_NO_RANDOMIZE; + assert_se(opinionated_personality(¤t_opinionated) >= 0); - log_info("current personality=0x%lX", current); + log_info("current personality=0x%lX", (unsigned long) safe_personality(PERSONALITY_INVALID)); + log_info("current opinionated personality=0x%lX", current_opinionated); pid = fork(); assert_se(pid >= 0); if (pid == 0) { - assert_se(seccomp_lock_personality(current) >= 0); + unsigned long current; - assert_se((unsigned long) safe_personality(current) == current); + assert_se(seccomp_lock_personality(current_opinionated) >= 0); + + current = safe_personality(current_opinionated); + assert_se((current & OPINIONATED_PERSONALITY_MASK) == current_opinionated); /* Note, we also test that safe_personality() works correctly, by checking whether errno is properly * set, in addition to the return value */ @@ -981,14 +978,15 @@ TEST(lock_personality) { assert_se(safe_personality(PER_LINUX_32BIT) == -EPERM); assert_se(safe_personality(PER_SVR4) == -EPERM); assert_se(safe_personality(PER_BSD) == -EPERM); - assert_se(safe_personality(current == PER_LINUX ? PER_LINUX32 : PER_LINUX) == -EPERM); + assert_se(safe_personality(current_opinionated == PER_LINUX ? PER_LINUX32 : PER_LINUX) == -EPERM); assert_se(safe_personality(PER_LINUX32_3GB) == -EPERM); assert_se(safe_personality(PER_UW7) == -EPERM); assert_se(safe_personality(0x42) == -EPERM); assert_se(safe_personality(PERSONALITY_INVALID) == -EPERM); /* maybe remove this later */ - assert_se((unsigned long) personality(current) == current); + current = safe_personality(current_opinionated); + assert_se((current & OPINIONATED_PERSONALITY_MASK) == current_opinionated); _exit(EXIT_SUCCESS); } From 14ce9b4ed8c8f05037611d68185c2db5367b3e42 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Wed, 7 Feb 2024 19:12:22 +0100 Subject: [PATCH 2/2] process-util: unify the hex value format a bit And fix the man page reference as well. --- src/basic/process-util.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/basic/process-util.h b/src/basic/process-util.h index 7b2c6cd679..9ca6753334 100644 --- a/src/basic/process-util.h +++ b/src/basic/process-util.h @@ -101,10 +101,10 @@ bool is_main_thread(void); bool oom_score_adjust_is_valid(int oa); #ifndef PERSONALITY_INVALID -/* personality(7) documents that 0xffffffffUL is used for querying the +/* personality(2) documents that 0xFFFFFFFFUL is used for querying the * current personality, hence let's use that here as error * indicator. */ -#define PERSONALITY_INVALID 0xffffffffLU +#define PERSONALITY_INVALID 0xFFFFFFFFUL #endif /* The personality() syscall returns a 32-bit value where the top three bytes are reserved for flags that