util-lib: wrap personality() to fix up broken glibc error handling (#6766)

glibc appears to propagate different errors in different ways, let's fix
this up, so that our own code doesn't get confused by this.

See #6752 + #6737 for details.

Fixes: #6755
This commit is contained in:
Lennart Poettering 2017-09-08 16:16:29 +02:00 committed by Evgeny Vereshchagin
parent 5463fa0a88
commit 21022b9dde
5 changed files with 53 additions and 57 deletions

View File

@ -904,6 +904,28 @@ const char* personality_to_string(unsigned long p) {
return architecture_to_string(architecture);
}
int safe_personality(unsigned long p) {
int ret;
/* So here's the deal, personality() is weirdly defined by glibc. In some cases it returns a failure via errno,
* and in others as negative return value containing an errno-like value. Let's work around this: this is a
* wrapper that uses errno if it is set, and uses the return value otherwise. And then it sets both errno and
* the return value indicating the same issue, so that we are definitely on the safe side.
*
* See https://github.com/systemd/systemd/issues/6737 */
errno = 0;
ret = personality(p);
if (ret < 0) {
if (errno != 0)
return -errno;
errno = -ret;
}
return ret;
}
int opinionated_personality(unsigned long *ret) {
int current;
@ -911,9 +933,9 @@ int opinionated_personality(unsigned long *ret) {
* opinionated though, and ignores all the finer-grained bits and exotic personalities, only distinguishing the
* two most relevant personalities: PER_LINUX and PER_LINUX32. */
current = personality(PERSONALITY_INVALID);
current = safe_personality(PERSONALITY_INVALID);
if (current < 0)
return -errno;
return current;
if (((unsigned long) current & 0xffff) == PER_LINUX32)
*ret = PER_LINUX32;

View File

@ -91,6 +91,7 @@ bool oom_score_adjust_is_valid(int oa);
unsigned long personality_from_string(const char *p);
const char *personality_to_string(unsigned long);
int safe_personality(unsigned long p);
int opinionated_personality(unsigned long *ret);
int ioprio_class_to_string_alloc(int i, char **s);

View File

@ -2614,11 +2614,13 @@ static int exec_child(
return -errno;
}
if (context->personality != PERSONALITY_INVALID)
if (personality(context->personality) < 0) {
if (context->personality != PERSONALITY_INVALID) {
r = safe_personality(context->personality);
if (r < 0) {
*exit_status = EXIT_PERSONALITY;
return -errno;
return r;
}
}
if (context->utmp_id)
utmp_put_init_process(context->utmp_id, getpid_cached(), getsid(0),

View File

@ -2267,11 +2267,13 @@ static int inner_child(
setup_hostname();
if (arg_personality != PERSONALITY_INVALID) {
if (personality(arg_personality) < 0)
return log_error_errno(errno, "personality() failed: %m");
r = safe_personality(arg_personality);
if (r < 0)
return log_error_errno(r, "personality() failed: %m");
} else if (secondary) {
if (personality(PER_LINUX32) < 0)
return log_error_errno(errno, "personality() failed: %m");
r = safe_personality(PER_LINUX32);
if (r < 0)
return log_error_errno(r, "personality() failed: %m");
}
#ifdef HAVE_SELINUX

View File

@ -582,59 +582,28 @@ static void test_lock_personality(void) {
assert_se(pid >= 0);
if (pid == 0) {
int ret;
assert_se(seccomp_lock_personality(current) >= 0);
assert_se((unsigned long) personality(current) == current);
assert_se((unsigned long) safe_personality(current) == current);
errno = EUCLEAN;
ret = personality(PER_LINUX | ADDR_NO_RANDOMIZE);
assert_se((ret == -1 && errno == EPERM) || (ret == -EPERM && errno == EUCLEAN));
/* Note, we also test that safe_personality() works correctly, by checkig whether errno is properly
* set, in addition to the return value */
errno = 0;
assert_se(safe_personality(PER_LINUX | ADDR_NO_RANDOMIZE) == -EPERM);
assert_se(errno == EPERM);
errno = EUCLEAN;
ret = personality(PER_LINUX | MMAP_PAGE_ZERO);
assert_se((ret == -1 && errno == EPERM) || (ret == -EPERM && errno == EUCLEAN));
assert_se(safe_personality(PER_LINUX | MMAP_PAGE_ZERO) == -EPERM);
assert_se(safe_personality(PER_LINUX | ADDR_COMPAT_LAYOUT) == -EPERM);
assert_se(safe_personality(PER_LINUX | READ_IMPLIES_EXEC) == -EPERM);
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(PER_LINUX32_3GB) == -EPERM);
assert_se(safe_personality(PER_UW7) == -EPERM);
assert_se(safe_personality(0x42) == -EPERM);
errno = EUCLEAN;
ret = personality(PER_LINUX | ADDR_COMPAT_LAYOUT);
assert_se((ret == -1 && errno == EPERM) || (ret == -EPERM && errno == EUCLEAN));
errno = EUCLEAN;
ret = personality(PER_LINUX | READ_IMPLIES_EXEC);
assert_se((ret == -1 && errno == EPERM) || (ret == -EPERM && errno == EUCLEAN));
errno = EUCLEAN;
ret = personality(PER_LINUX_32BIT);
assert_se((ret == -1 && errno == EPERM) || (ret == -EPERM && errno == EUCLEAN));
errno = EUCLEAN;
ret = personality(PER_SVR4);
assert_se((ret == -1 && errno == EPERM) || (ret == -EPERM && errno == EUCLEAN));
errno = EUCLEAN;
ret = personality(PER_BSD);
assert_se((ret == -1 && errno == EPERM) || (ret == -EPERM && errno == EUCLEAN));
errno = EUCLEAN;
ret = personality(current == PER_LINUX ? PER_LINUX32 : PER_LINUX);
assert_se((ret == -1 && errno == EPERM) || (ret == -EPERM && errno == EUCLEAN));
errno = EUCLEAN;
ret = personality(PER_LINUX32_3GB);
assert_se((ret == -1 && errno == EPERM) || (ret == -EPERM && errno == EUCLEAN));
errno = EUCLEAN;
ret = personality(PER_UW7);
assert_se((ret == -1 && errno == EPERM) || (ret == -EPERM && errno == EUCLEAN));
errno = EUCLEAN;
ret = personality(0x42);
assert_se((ret == -1 && errno == EPERM) || (ret == -EPERM && errno == EUCLEAN));
errno = EUCLEAN;
ret = personality(PERSONALITY_INVALID); /* maybe remove this later */
assert_se((ret == -1 && errno == EPERM) || (ret == -EPERM && errno == EUCLEAN));
assert_se(safe_personality(PERSONALITY_INVALID) == -EPERM); /* maybe remove this later */
assert_se((unsigned long) personality(current) == current);
_exit(EXIT_SUCCESS);